Skip to content

Add exportname option to CLI and interop tests#134

Merged
gaborigloi merged 2 commits into
xapi-project:masterfrom
gaborigloi:interop_tests
Aug 9, 2018
Merged

Add exportname option to CLI and interop tests#134
gaborigloi merged 2 commits into
xapi-project:masterfrom
gaborigloi:interop_tests

Conversation

@gaborigloi
Copy link
Copy Markdown
Contributor

Add an option to specify an export name when serving a file. If
specified, only this export name will be accepted, and the client will
be allowed to list the exports.

If unspecified, the behavior is the same as before: listing the exports
is not allowed, and any export name is accepted.

I've added compatibility tests with qemu-img and nbd-client. By default,
these test are skipped if the required programs aren't installed. If the
strict option is enabled, the tests will fail instead of being skipped.

Signed-off-by: Gabor Igloi gabor.igloi@citrix.com

@gaborigloi gaborigloi force-pushed the interop_tests branch 4 times, most recently from 0542214 to 924df45 Compare August 9, 2018 12:46
This is better, as we directly connect to the server, instead of going
through an additional component (nbd-client), and we don't even need
sudo access.

Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
@gaborigloi gaborigloi force-pushed the interop_tests branch 2 times, most recently from 3a2398b to af0c1d6 Compare August 9, 2018 13:22
@gaborigloi
Copy link
Copy Markdown
Contributor Author

Travis build depends on ocaml/ocaml-ci-scripts#237

@gaborigloi gaborigloi force-pushed the interop_tests branch 2 times, most recently from a0b2016 to 71f7bc4 Compare August 9, 2018 13:58
Comment thread benchmark.sh
SERVER_PROCESS=$!
echo $SERVER_PROCESS
# Wait for the server to start the main loop
sleep 0.1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there some way the server could notify that it has started?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there might be, maybe we could wait until something is listening on the 10809 port, but I wanted to keep things simple in the tests.

Comment thread cli/main.ml Outdated
Server.with_connection
(match exportname with
| None -> Server.with_connection ?offer:None
| Some exportname -> Server.with_connection ~offer:[exportname])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be written as:

let offer = match exportname with 
| None -> None
| Some exportname -> Some [exportname] in
Server.with_connection ?offer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done

@gaborigloi gaborigloi force-pushed the interop_tests branch 3 times, most recently from 1cd36ea to ab84d57 Compare August 9, 2018 17:42
Add an option to specify an export name when serving a file. If
specified, only this export name will be accepted, and the client will
be allowed to list the exports.

If unspecified, the behavior is the same as before: listing the exports
is not allowed, and any export name is accepted.

I've added compatibility tests with qemu-img and nbd-client. By default,
these test are skipped if the required programs aren't installed. If the
strict option is enabled, the tests will fail instead of being skipped.

Signed-off-by: Gabor Igloi <gabor.igloi@citrix.com>
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 74.643% when pulling 381c515 on gaborigloi:interop_tests into 9935761 on xapi-project:master.

@gaborigloi gaborigloi merged commit 20e39ca into xapi-project:master Aug 9, 2018
@gaborigloi gaborigloi deleted the interop_tests branch August 9, 2018 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants