| Message ID | 1478566785-4002-3-git-send-email-ashish.mittal@veritas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: > These changes use a vxhs test server that is a part of the following > repository: > https://github.com/MittalAshish/libqnio.git > > Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> > --- > v6 changelog: > (1) Added iotests for VxHS block device. > > tests/qemu-iotests/common | 6 ++++++ > tests/qemu-iotests/common.config | 13 +++++++++++++ > tests/qemu-iotests/common.filter | 1 + > tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ > 4 files changed, 39 insertions(+) > > diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common > index d60ea2c..41430d8 100644 > --- a/tests/qemu-iotests/common > +++ b/tests/qemu-iotests/common When using raw format, I was able to run the test successfully for all supported test cases (26 of them). With qcow2, they fail - but not the fault of this patch, I think; but rather, the fault of the test server. Can qnio_server be modified so that it does not work on just raw files? > @@ -158,6 +158,7 @@ check options > -nfs test nfs > -archipelago test archipelago > -luks test luks > + -vxhs test vxhs > -xdiff graphical mode diff > -nocache use O_DIRECT on backing file > -misalign misalign memory allocations > @@ -261,6 +262,11 @@ testlist options > xpand=false > ;; > > + -vxhs) > + IMGPROTO=vxhs > + xpand=false > + ;; > + > -ssh) > IMGPROTO=ssh > xpand=false > diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config > index f6384fb..c7a80c0 100644 > --- a/tests/qemu-iotests/common.config > +++ b/tests/qemu-iotests/common.config > @@ -105,6 +105,10 @@ if [ -z "$QEMU_NBD_PROG" ]; then > export QEMU_NBD_PROG="`set_prog_path qemu-nbd`" > fi > > +if [ -z "$QEMU_VXHS_PROG" ]; then > + export QEMU_VXHS_PROG="`set_prog_path qnio_server /usr/local/bin`" > +fi > + > _qemu_wrapper() > { > ( > @@ -156,10 +160,19 @@ _qemu_nbd_wrapper() > ) > } > > +_qemu_vxhs_wrapper() > +{ > + ( > + echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid" > + exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@" > + ) > +} > + > export QEMU=_qemu_wrapper > export QEMU_IMG=_qemu_img_wrapper > export QEMU_IO=_qemu_io_wrapper > export QEMU_NBD=_qemu_nbd_wrapper > +export QEMU_VXHS=_qemu_vxhs_wrapper > > QEMU_IMG_EXTRA_ARGS= > if [ "$IMGOPTSSYNTAX" = "true" ]; then > diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter > index 240ed06..a8a4d0e 100644 > --- a/tests/qemu-iotests/common.filter > +++ b/tests/qemu-iotests/common.filter > @@ -123,6 +123,7 @@ _filter_img_info() > -e "s#$TEST_DIR#TEST_DIR#g" \ > -e "s#$IMGFMT#IMGFMT#g" \ > -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \ > + -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \ > -e "/encrypted: yes/d" \ > -e "/cluster_size: [0-9]\\+/d" \ > -e "/table_size: [0-9]\\+/d" \ > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc > index 3213765..06a3164 100644 > --- a/tests/qemu-iotests/common.rc > +++ b/tests/qemu-iotests/common.rc > @@ -89,6 +89,9 @@ else > TEST_IMG=$TEST_DIR/t.$IMGFMT > elif [ "$IMGPROTO" = "archipelago" ]; then > TEST_IMG="archipelago:at.$IMGFMT" > + elif [ "$IMGPROTO" = "vxhs" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > + TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT" > else > TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT > fi > @@ -175,6 +178,12 @@ _make_test_img() > eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT $TEST_IMG_FILE &" > sleep 1 # FIXME: qemu-nbd needs to be listening before we continue > fi > + > + # Start QNIO server on image directory for vxhs protocol > + if [ $IMGPROTO = "vxhs" ]; then > + eval "$QEMU_VXHS -d $TEST_DIR &" > + sleep 1 # Wait for server to come up. > + fi > } > > _rm_test_img() > @@ -201,6 +210,16 @@ _cleanup_test_img() > fi > rm -f "$TEST_IMG_FILE" > ;; > + vxhs) > + if [ -f "${TEST_DIR}/qemu-vxhs.pid" ]; then > + local QEMU_VXHS_PID > + read QEMU_VXHS_PID < "${TEST_DIR}/qemu-vxhs.pid" > + kill ${QEMU_VXHS_PID} >/dev/null 2>&1 > + rm -f "${TEST_DIR}/qemu-vxhs.pid" > + fi > + rm -f "$TEST_IMG_FILE" > + ;; > + > file) > _rm_test_img "$TEST_DIR/t.$IMGFMT" > _rm_test_img "$TEST_DIR/t.$IMGFMT.orig" > -- > 1.8.3.1 >
I plan to submit v7 soon, therefore going over any unanswered emails at this time. On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody <jcody@redhat.com> wrote: > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: >> These changes use a vxhs test server that is a part of the following >> repository: >> https://github.com/MittalAshish/libqnio.git >> >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> >> --- >> v6 changelog: >> (1) Added iotests for VxHS block device. >> >> tests/qemu-iotests/common | 6 ++++++ >> tests/qemu-iotests/common.config | 13 +++++++++++++ >> tests/qemu-iotests/common.filter | 1 + >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ >> 4 files changed, 39 insertions(+) >> >> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common >> index d60ea2c..41430d8 100644 >> --- a/tests/qemu-iotests/common >> +++ b/tests/qemu-iotests/common > > When using raw format, I was able to run the test successfully for all > supported test cases (26 of them). > > With qcow2, they fail - but not the fault of this patch, I think; but > rather, the fault of the test server. Can qnio_server be modified so that > it does not work on just raw files? > > VxHS supports and uses only the raw format. > >> @@ -158,6 +158,7 @@ check options >> -nfs test nfs >> -archipelago test archipelago >> -luks test luks >> + -vxhs test vxhs >> -xdiff graphical mode diff >> -nocache use O_DIRECT on backing file >> -misalign misalign memory allocations >> @@ -261,6 +262,11 @@ testlist options >> xpand=false >> ;; >> >> + -vxhs) >> + IMGPROTO=vxhs >> + xpand=false >> + ;; >> + >> -ssh) >> IMGPROTO=ssh >> xpand=false >> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config >> index f6384fb..c7a80c0 100644 >> --- a/tests/qemu-iotests/common.config >> +++ b/tests/qemu-iotests/common.config >> @@ -105,6 +105,10 @@ if [ -z "$QEMU_NBD_PROG" ]; then >> export QEMU_NBD_PROG="`set_prog_path qemu-nbd`" >> fi >> >> +if [ -z "$QEMU_VXHS_PROG" ]; then >> + export QEMU_VXHS_PROG="`set_prog_path qnio_server /usr/local/bin`" >> +fi >> + >> _qemu_wrapper() >> { >> ( >> @@ -156,10 +160,19 @@ _qemu_nbd_wrapper() >> ) >> } >> >> +_qemu_vxhs_wrapper() >> +{ >> + ( >> + echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid" >> + exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@" >> + ) >> +} >> + >> export QEMU=_qemu_wrapper >> export QEMU_IMG=_qemu_img_wrapper >> export QEMU_IO=_qemu_io_wrapper >> export QEMU_NBD=_qemu_nbd_wrapper >> +export QEMU_VXHS=_qemu_vxhs_wrapper >> >> QEMU_IMG_EXTRA_ARGS= >> if [ "$IMGOPTSSYNTAX" = "true" ]; then >> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter >> index 240ed06..a8a4d0e 100644 >> --- a/tests/qemu-iotests/common.filter >> +++ b/tests/qemu-iotests/common.filter >> @@ -123,6 +123,7 @@ _filter_img_info() >> -e "s#$TEST_DIR#TEST_DIR#g" \ >> -e "s#$IMGFMT#IMGFMT#g" \ >> -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \ >> + -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \ >> -e "/encrypted: yes/d" \ >> -e "/cluster_size: [0-9]\\+/d" \ >> -e "/table_size: [0-9]\\+/d" \ >> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc >> index 3213765..06a3164 100644 >> --- a/tests/qemu-iotests/common.rc >> +++ b/tests/qemu-iotests/common.rc >> @@ -89,6 +89,9 @@ else >> TEST_IMG=$TEST_DIR/t.$IMGFMT >> elif [ "$IMGPROTO" = "archipelago" ]; then >> TEST_IMG="archipelago:at.$IMGFMT" >> + elif [ "$IMGPROTO" = "vxhs" ]; then >> + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT >> + TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT" >> else >> TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT >> fi >> @@ -175,6 +178,12 @@ _make_test_img() >> eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT $TEST_IMG_FILE &" >> sleep 1 # FIXME: qemu-nbd needs to be listening before we continue >> fi >> + >> + # Start QNIO server on image directory for vxhs protocol >> + if [ $IMGPROTO = "vxhs" ]; then >> + eval "$QEMU_VXHS -d $TEST_DIR &" >> + sleep 1 # Wait for server to come up. >> + fi >> } >> >> _rm_test_img() >> @@ -201,6 +210,16 @@ _cleanup_test_img() >> fi >> rm -f "$TEST_IMG_FILE" >> ;; >> + vxhs) >> + if [ -f "${TEST_DIR}/qemu-vxhs.pid" ]; then >> + local QEMU_VXHS_PID >> + read QEMU_VXHS_PID < "${TEST_DIR}/qemu-vxhs.pid" >> + kill ${QEMU_VXHS_PID} >/dev/null 2>&1 >> + rm -f "${TEST_DIR}/qemu-vxhs.pid" >> + fi >> + rm -f "$TEST_IMG_FILE" >> + ;; >> + >> file) >> _rm_test_img "$TEST_DIR/t.$IMGFMT" >> _rm_test_img "$TEST_DIR/t.$IMGFMT.orig" >> -- >> 1.8.3.1 >>
On Tue, Feb 07, 2017 at 03:12:36PM -0800, ashish mittal wrote: > On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody <jcody@redhat.com> wrote: > > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: > >> These changes use a vxhs test server that is a part of the following > >> repository: > >> https://github.com/MittalAshish/libqnio.git > >> > >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> > >> --- > >> v6 changelog: > >> (1) Added iotests for VxHS block device. > >> > >> tests/qemu-iotests/common | 6 ++++++ > >> tests/qemu-iotests/common.config | 13 +++++++++++++ > >> tests/qemu-iotests/common.filter | 1 + > >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ > >> 4 files changed, 39 insertions(+) > >> > >> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common > >> index d60ea2c..41430d8 100644 > >> --- a/tests/qemu-iotests/common > >> +++ b/tests/qemu-iotests/common > > > > When using raw format, I was able to run the test successfully for all > > supported test cases (26 of them). > > > > With qcow2, they fail - but not the fault of this patch, I think; but > > rather, the fault of the test server. Can qnio_server be modified so that > > it does not work on just raw files? > > > > > > VxHS supports and uses only the raw format. That's like saying only ext4 guest file systems are supported on VxHS and not ZFS. The VxHS driver should not care what file system is used, it just does block I/O without interpreting the data. It must be possible to use any format on top of the VxHS protocol. After all, the image format drivers just do block I/O. If there is a case where qcow2 on VxHS fails then it needs to be investigated. The VxHS driver can't be merged until we at least understand the cause of the qcow2 test failures. Stefan
On Mon, Feb 13, 2017 at 01:37:25PM +0000, Stefan Hajnoczi wrote: > On Tue, Feb 07, 2017 at 03:12:36PM -0800, ashish mittal wrote: > > On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody <jcody@redhat.com> wrote: > > > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: > > >> These changes use a vxhs test server that is a part of the following > > >> repository: > > >> https://github.com/MittalAshish/libqnio.git > > >> > > >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> > > >> --- > > >> v6 changelog: > > >> (1) Added iotests for VxHS block device. > > >> > > >> tests/qemu-iotests/common | 6 ++++++ > > >> tests/qemu-iotests/common.config | 13 +++++++++++++ > > >> tests/qemu-iotests/common.filter | 1 + > > >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ > > >> 4 files changed, 39 insertions(+) > > >> > > >> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common > > >> index d60ea2c..41430d8 100644 > > >> --- a/tests/qemu-iotests/common > > >> +++ b/tests/qemu-iotests/common > > > > > > When using raw format, I was able to run the test successfully for all > > > supported test cases (26 of them). > > > > > > With qcow2, they fail - but not the fault of this patch, I think; but > > > rather, the fault of the test server. Can qnio_server be modified so that > > > it does not work on just raw files? > > > > > > > > > > VxHS supports and uses only the raw format. > > That's like saying only ext4 guest file systems are supported on VxHS > and not ZFS. The VxHS driver should not care what file system is used, > it just does block I/O without interpreting the data. > > It must be possible to use any format on top of the VxHS protocol. > After all, the image format drivers just do block I/O. If there is a > case where qcow2 on VxHS fails then it needs to be investigated. > > The VxHS driver can't be merged until we at least understand the cause > of the qcow2 test failures. > A quick run with the test server and a QEMU process showed an abort() in the test server, so I just sent a pull req to libqnio to fix that. But playing with it in gdb right now with a test qcow2 file, I see that we are waiting in aio_poll() forever for the test server to respond to a read request, when using qcow2 format. As Stefan said, this doesn't really make any sense - why would VXHS behave differently based on the file contents? Jeff
On 2/13/17, 8:32 AM, "Jeff Cody" <jcody@redhat.com> wrote:
On Mon, Feb 13, 2017 at 01:37:25PM +0000, Stefan Hajnoczi wrote:
> On Tue, Feb 07, 2017 at 03:12:36PM -0800, ashish mittal wrote:
> > On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody <jcody@redhat.com> wrote:
> > > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote:
> > >> These changes use a vxhs test server that is a part of the following
> > >> repository:
> > >> https://github.com/MittalAshish/libqnio.git
> > >>
> > >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com>
> > >> ---
> > >> v6 changelog:
> > >> (1) Added iotests for VxHS block device.
> > >>
> > >> tests/qemu-iotests/common | 6 ++++++
> > >> tests/qemu-iotests/common.config | 13 +++++++++++++
> > >> tests/qemu-iotests/common.filter | 1 +
> > >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++
> > >> 4 files changed, 39 insertions(+)
> > >>
> > >> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
> > >> index d60ea2c..41430d8 100644
> > >> --- a/tests/qemu-iotests/common
> > >> +++ b/tests/qemu-iotests/common
> > >
> > > When using raw format, I was able to run the test successfully for all
> > > supported test cases (26 of them).
> > >
> > > With qcow2, they fail - but not the fault of this patch, I think; but
> > > rather, the fault of the test server. Can qnio_server be modified so that
> > > it does not work on just raw files?
> > >
> > >
> >
> > VxHS supports and uses only the raw format.
>
> That's like saying only ext4 guest file systems are supported on VxHS
> and not ZFS. The VxHS driver should not care what file system is used,
> it just does block I/O without interpreting the data.
>
> It must be possible to use any format on top of the VxHS protocol.
> After all, the image format drivers just do block I/O. If there is a
> case where qcow2 on VxHS fails then it needs to be investigated.
>
> The VxHS driver can't be merged until we at least understand the cause
> of the qcow2 test failures.
>
A quick run with the test server and a QEMU process showed an abort() in the
test server, so I just sent a pull req to libqnio to fix that.
But playing with it in gdb right now with a test qcow2 file, I see that we
are waiting in aio_poll() forever for the test server to respond to a read
request, when using qcow2 format.
As Stefan said, this doesn't really make any sense - why would VXHS behave
differently based on the file contents?
[Ketan]
To read/write a qcow2 backed device VxHS server implementation will need to understand the qcow2 format. This is not just block IO but actually does involve interpreting the qcow2 header and cluster formats. Clearly the test server implementation does not handle it as it was never intended to. VxHS backend won't handle it either because VxHS virtual disks are written as non-sparse files.
There are space savings with the qcow2 format but performance penalties as well because of the metadata overhead. As a block storage provider, VxHS does not support sparse file formats like qcow2 primarily because of performance reasons.
Implementing a qcow2 backend in the test server would be a non-trivial and non-useful exercise since the VxHS server won't support it.
Jeff
On Mon, Feb 13, 2017 at 10:36:53PM +0000, Ketan Nilangekar wrote: > > > On 2/13/17, 8:32 AM, "Jeff Cody" <jcody@redhat.com> wrote: > > On Mon, Feb 13, 2017 at 01:37:25PM +0000, Stefan Hajnoczi wrote: > > On Tue, Feb 07, 2017 at 03:12:36PM -0800, ashish mittal wrote: > > > On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody <jcody@redhat.com> wrote: > > > > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: > > > >> These changes use a vxhs test server that is a part of the following > > > >> repository: > > > >> https://github.com/MittalAshish/libqnio.git > > > >> > > > >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> > > > >> --- > > > >> v6 changelog: > > > >> (1) Added iotests for VxHS block device. > > > >> > > > >> tests/qemu-iotests/common | 6 ++++++ > > > >> tests/qemu-iotests/common.config | 13 +++++++++++++ > > > >> tests/qemu-iotests/common.filter | 1 + > > > >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ > > > >> 4 files changed, 39 insertions(+) > > > >> > > > >> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common > > > >> index d60ea2c..41430d8 100644 > > > >> --- a/tests/qemu-iotests/common > > > >> +++ b/tests/qemu-iotests/common > > > > > > > > When using raw format, I was able to run the test successfully for all > > > > supported test cases (26 of them). > > > > > > > > With qcow2, they fail - but not the fault of this patch, I think; but > > > > rather, the fault of the test server. Can qnio_server be modified so that > > > > it does not work on just raw files? > > > > > > > > > > > > > > VxHS supports and uses only the raw format. > > > > That's like saying only ext4 guest file systems are supported on VxHS > > and not ZFS. The VxHS driver should not care what file system is used, > > it just does block I/O without interpreting the data. > > > > It must be possible to use any format on top of the VxHS protocol. > > After all, the image format drivers just do block I/O. If there is a > > case where qcow2 on VxHS fails then it needs to be investigated. > > > > The VxHS driver can't be merged until we at least understand the cause > > of the qcow2 test failures. > > > > A quick run with the test server and a QEMU process showed an abort() in the > test server, so I just sent a pull req to libqnio to fix that. > > But playing with it in gdb right now with a test qcow2 file, I see that we > are waiting in aio_poll() forever for the test server to respond to a read > request, when using qcow2 format. > > As Stefan said, this doesn't really make any sense - why would VXHS behave > differently based on the file contents? > > [Ketan] To read/write a qcow2 backed device VxHS server implementation > will need to understand the qcow2 format. This is not just block IO but > actually does involve interpreting the qcow2 header and cluster formats. > Clearly the test server implementation does not handle it as it was never > intended to. VxHS backend won't handle it either because VxHS virtual > disks are written as non-sparse files. There are space savings with the > qcow2 format but performance penalties as well because of the metadata > overhead. As a block storage provider, VxHS does not support sparse file > formats like qcow2 primarily because of performance reasons. Implementing > a qcow2 backend in the test server would be a non-trivial and non-useful > exercise since the VxHS server won't support it. > What? Why would the backend need to know anything about qcow2 formats; are you manipulating the guest image data directly yourself? But regardless, since the test server is naive and just reads and writes data, the fact that the test server breaks on qcow2 image formats means that the test server is broken for raw images, as well [1]. The backend should not need to know anything about the image file format in order to serve data, as the backend is essentially serving bytes. (I guess the only thing the backend would need to be aware of is how to invoke qemu-img to create a qcow2 image file initially). QEMU is what interprets the qcow2 format (or any image format). Every read from QEMU will look like a raw file read from the perspective of libqnio / vxhs. I can't see any reason why vxhs would need to know anything about the contents of the image file itself. The only thing the test server needs to do, in order to be able to serve qcow2 files correctly, is to be able to serve raw files correctly. Looking at the libqnio implementation from the test server with gdb, I think that the reason why qcow2 format does not work is that the current libqnio implementation does not handle short reads correctly. For instance, if the file is not a even multiple of the read size, and we try to read 512 bytes at the end of the file, it appears libqnio hangs on the short read. I am not sure if this is a bug exclusive to the test server, or in the libqnio implementation. [1] This bug isn't limited to qcow2, since as mentioned above, QEMU is just relying on libqnio to read raw bytes. So, you can reproduce this bug with a raw image file, as well: # dd if=/dev/zero of=test.raw bs=1 count=196616 # qemu-io -c "read 196608 512" vxhs://localhost:9999/test.raw The above qemu-io process will hang in aio_poll, because libqnio never processes the callback. This is because process_incoming_messages() thinks it needs a full 512 bytes before it will call the registered callback, even though there will only ever be 8 bytes read. The above scenario is also exactly what is happening when I try to use a qcow2 format; QEMU issues a read for 8 bytes at offset 196608, and the file is 196616 bytes long. (The sparse image thing seems shouldn't be an issue - first, the test server works just fine with sparse raw images... as matter of fact, the "create_vdisk.sh" script in the libqnio repository does exactly that. Also, if you choose to not use sparse images for whatever reason on the backend, qcow2 does not require it to be sparse, as image files can be created with the "-o preallocation=full" option). -Jeff
On 2/13/17, 3:23 PM, "Jeff Cody" <jcody@redhat.com> wrote:
On Mon, Feb 13, 2017 at 10:36:53PM +0000, Ketan Nilangekar wrote:
>
>
> On 2/13/17, 8:32 AM, "Jeff Cody" <jcody@redhat.com> wrote:
>
> On Mon, Feb 13, 2017 at 01:37:25PM +0000, Stefan Hajnoczi wrote:
> > On Tue, Feb 07, 2017 at 03:12:36PM -0800, ashish mittal wrote:
> > > On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody <jcody@redhat.com> wrote:
> > > > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote:
> > > >> These changes use a vxhs test server that is a part of the following
> > > >> repository:
> > > >> https://github.com/MittalAshish/libqnio.git
> > > >>
> > > >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com>
> > > >> ---
> > > >> v6 changelog:
> > > >> (1) Added iotests for VxHS block device.
> > > >>
> > > >> tests/qemu-iotests/common | 6 ++++++
> > > >> tests/qemu-iotests/common.config | 13 +++++++++++++
> > > >> tests/qemu-iotests/common.filter | 1 +
> > > >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++
> > > >> 4 files changed, 39 insertions(+)
> > > >>
> > > >> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
> > > >> index d60ea2c..41430d8 100644
> > > >> --- a/tests/qemu-iotests/common
> > > >> +++ b/tests/qemu-iotests/common
> > > >
> > > > When using raw format, I was able to run the test successfully for all
> > > > supported test cases (26 of them).
> > > >
> > > > With qcow2, they fail - but not the fault of this patch, I think; but
> > > > rather, the fault of the test server. Can qnio_server be modified so that
> > > > it does not work on just raw files?
> > > >
> > > >
> > >
> > > VxHS supports and uses only the raw format.
> >
> > That's like saying only ext4 guest file systems are supported on VxHS
> > and not ZFS. The VxHS driver should not care what file system is used,
> > it just does block I/O without interpreting the data.
> >
> > It must be possible to use any format on top of the VxHS protocol.
> > After all, the image format drivers just do block I/O. If there is a
> > case where qcow2 on VxHS fails then it needs to be investigated.
> >
> > The VxHS driver can't be merged until we at least understand the cause
> > of the qcow2 test failures.
> >
>
> A quick run with the test server and a QEMU process showed an abort() in the
> test server, so I just sent a pull req to libqnio to fix that.
>
> But playing with it in gdb right now with a test qcow2 file, I see that we
> are waiting in aio_poll() forever for the test server to respond to a read
> request, when using qcow2 format.
>
> As Stefan said, this doesn't really make any sense - why would VXHS behave
> differently based on the file contents?
>
> [Ketan] To read/write a qcow2 backed device VxHS server implementation
> will need to understand the qcow2 format. This is not just block IO but
> actually does involve interpreting the qcow2 header and cluster formats.
> Clearly the test server implementation does not handle it as it was never
> intended to. VxHS backend won't handle it either because VxHS virtual
> disks are written as non-sparse files. There are space savings with the
> qcow2 format but performance penalties as well because of the metadata
> overhead. As a block storage provider, VxHS does not support sparse file
> formats like qcow2 primarily because of performance reasons. Implementing
> a qcow2 backend in the test server would be a non-trivial and non-useful
> exercise since the VxHS server won't support it.
>
What? Why would the backend need to know anything about qcow2 formats; are
you manipulating the guest image data directly yourself? But regardless,
since the test server is naive and just reads and writes data, the fact that
the test server breaks on qcow2 image formats means that the test server is
broken for raw images, as well [1].
The backend should not need to know anything about the image file format in
order to serve data, as the backend is essentially serving bytes. (I guess
the only thing the backend would need to be aware of is how to invoke
qemu-img to create a qcow2 image file initially).
QEMU is what interprets the qcow2 format (or any image format). Every read
from QEMU will look like a raw file read from the perspective of libqnio /
vxhs. I can't see any reason why vxhs would need to know anything about the
contents of the image file itself.
The only thing the test server needs to do, in order to be able to serve
qcow2 files correctly, is to be able to serve raw files correctly.
Looking at the libqnio implementation from the test server with gdb, I think
that the reason why qcow2 format does not work is that the current
libqnio implementation does not handle short reads correctly. For instance,
if the file is not a even multiple of the read size, and we try to read 512
bytes at the end of the file, it appears libqnio hangs on the short read. I
am not sure if this is a bug exclusive to the test server, or in the libqnio
implementation.
[1]
This bug isn't limited to qcow2, since as mentioned above, QEMU is just
relying on libqnio to read raw bytes. So, you can reproduce this bug with a
raw image file, as well:
# dd if=/dev/zero of=test.raw bs=1 count=196616
# qemu-io -c "read 196608 512" vxhs://localhost:9999/test.raw
The above qemu-io process will hang in aio_poll, because libqnio never
processes the callback. This is because process_incoming_messages() thinks
it needs a full 512 bytes before it will call the registered callback, even
though there will only ever be 8 bytes read.
The above scenario is also exactly what is happening when I try to use a
qcow2 format; QEMU issues a read for 8 bytes at offset 196608, and the file
is 196616 bytes long.
(The sparse image thing seems shouldn't be an issue - first, the test server
works just fine with sparse raw images... as matter of fact, the
"create_vdisk.sh" script in the libqnio repository does exactly that. Also,
if you choose to not use sparse images for whatever reason on the backend,
qcow2 does not require it to be sparse, as image files can be created with
the "-o preallocation=full" option).
[Ketan] You are correct. We have identified the problem with test server and have a patch for it which returns zero filled reads to align the read buffers to requested block size. We could also fix this in libvxhs by not expecting/waiting for requested block size in case the reads are partial.
Ashish will upload the patch for the test server fix. Please let us know how the qcow tests go.
-Jeff
On Mon, Feb 13, 2017 at 7:02 PM, Ketan Nilangekar <Ketan.Nilangekar@veritas.com> wrote: > > > On 2/13/17, 3:23 PM, "Jeff Cody" <jcody@redhat.com> wrote: > > On Mon, Feb 13, 2017 at 10:36:53PM +0000, Ketan Nilangekar wrote: > > > > > > On 2/13/17, 8:32 AM, "Jeff Cody" <jcody@redhat.com> wrote: > > > > On Mon, Feb 13, 2017 at 01:37:25PM +0000, Stefan Hajnoczi wrote: > > > On Tue, Feb 07, 2017 at 03:12:36PM -0800, ashish mittal wrote: > > > > On Tue, Nov 8, 2016 at 12:44 PM, Jeff Cody <jcody@redhat.com> wrote: > > > > > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: > > > > >> These changes use a vxhs test server that is a part of the following > > > > >> repository: > > > > >> https://github.com/MittalAshish/libqnio.git > > > > >> > > > > >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> > > > > >> --- > > > > >> v6 changelog: > > > > >> (1) Added iotests for VxHS block device. > > > > >> > > > > >> tests/qemu-iotests/common | 6 ++++++ > > > > >> tests/qemu-iotests/common.config | 13 +++++++++++++ > > > > >> tests/qemu-iotests/common.filter | 1 + > > > > >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ > > > > >> 4 files changed, 39 insertions(+) > > > > >> > > > > >> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common > > > > >> index d60ea2c..41430d8 100644 > > > > >> --- a/tests/qemu-iotests/common > > > > >> +++ b/tests/qemu-iotests/common > > > > > > > > > > When using raw format, I was able to run the test successfully for all > > > > > supported test cases (26 of them). > > > > > > > > > > With qcow2, they fail - but not the fault of this patch, I think; but > > > > > rather, the fault of the test server. Can qnio_server be modified so that > > > > > it does not work on just raw files? > > > > > > > > > > > > > > > > > > VxHS supports and uses only the raw format. > > > > > > That's like saying only ext4 guest file systems are supported on VxHS > > > and not ZFS. The VxHS driver should not care what file system is used, > > > it just does block I/O without interpreting the data. > > > > > > It must be possible to use any format on top of the VxHS protocol. > > > After all, the image format drivers just do block I/O. If there is a > > > case where qcow2 on VxHS fails then it needs to be investigated. > > > > > > The VxHS driver can't be merged until we at least understand the cause > > > of the qcow2 test failures. > > > > > > > A quick run with the test server and a QEMU process showed an abort() in the > > test server, so I just sent a pull req to libqnio to fix that. > > > > But playing with it in gdb right now with a test qcow2 file, I see that we > > are waiting in aio_poll() forever for the test server to respond to a read > > request, when using qcow2 format. > > > > As Stefan said, this doesn't really make any sense - why would VXHS behave > > differently based on the file contents? > > > > [Ketan] To read/write a qcow2 backed device VxHS server implementation > > will need to understand the qcow2 format. This is not just block IO but > > actually does involve interpreting the qcow2 header and cluster formats. > > Clearly the test server implementation does not handle it as it was never > > intended to. VxHS backend won't handle it either because VxHS virtual > > disks are written as non-sparse files. There are space savings with the > > qcow2 format but performance penalties as well because of the metadata > > overhead. As a block storage provider, VxHS does not support sparse file > > formats like qcow2 primarily because of performance reasons. Implementing > > a qcow2 backend in the test server would be a non-trivial and non-useful > > exercise since the VxHS server won't support it. > > > > What? Why would the backend need to know anything about qcow2 formats; are > you manipulating the guest image data directly yourself? But regardless, > since the test server is naive and just reads and writes data, the fact that > the test server breaks on qcow2 image formats means that the test server is > broken for raw images, as well [1]. > > The backend should not need to know anything about the image file format in > order to serve data, as the backend is essentially serving bytes. (I guess > the only thing the backend would need to be aware of is how to invoke > qemu-img to create a qcow2 image file initially). > > > QEMU is what interprets the qcow2 format (or any image format). Every read > from QEMU will look like a raw file read from the perspective of libqnio / > vxhs. I can't see any reason why vxhs would need to know anything about the > contents of the image file itself. > > The only thing the test server needs to do, in order to be able to serve > qcow2 files correctly, is to be able to serve raw files correctly. > > Looking at the libqnio implementation from the test server with gdb, I think > that the reason why qcow2 format does not work is that the current > libqnio implementation does not handle short reads correctly. For instance, > if the file is not a even multiple of the read size, and we try to read 512 > bytes at the end of the file, it appears libqnio hangs on the short read. I > am not sure if this is a bug exclusive to the test server, or in the libqnio > implementation. > > [1] > This bug isn't limited to qcow2, since as mentioned above, QEMU is just > relying on libqnio to read raw bytes. So, you can reproduce this bug with a > raw image file, as well: > > # dd if=/dev/zero of=test.raw bs=1 count=196616 > > # qemu-io -c "read 196608 512" vxhs://localhost:9999/test.raw > > The above qemu-io process will hang in aio_poll, because libqnio never > processes the callback. This is because process_incoming_messages() thinks > it needs a full 512 bytes before it will call the registered callback, even > though there will only ever be 8 bytes read. > > The above scenario is also exactly what is happening when I try to use a > qcow2 format; QEMU issues a read for 8 bytes at offset 196608, and the file > is 196616 bytes long. > > (The sparse image thing seems shouldn't be an issue - first, the test server > works just fine with sparse raw images... as matter of fact, the > "create_vdisk.sh" script in the libqnio repository does exactly that. Also, > if you choose to not use sparse images for whatever reason on the backend, > qcow2 does not require it to be sparse, as image files can be created with > the "-o preallocation=full" option). > > [Ketan] You are correct. We have identified the problem with test server and have a patch for it which returns zero filled reads to align the read buffers to requested block size. We could also fix this in libvxhs by not expecting/waiting for requested block size in case the reads are partial. > Ashish will upload the patch for the test server fix. Please let us know how the qcow tests go. > > -Jeff > > Changes per the above suggestion present in branch ashish_securify. Thanks!
On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: > These changes use a vxhs test server that is a part of the following > repository: > https://github.com/MittalAshish/libqnio.git > > Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> > --- > v6 changelog: > (1) Added iotests for VxHS block device. > > tests/qemu-iotests/common | 6 ++++++ > tests/qemu-iotests/common.config | 13 +++++++++++++ > tests/qemu-iotests/common.filter | 1 + > tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ > 4 files changed, 39 insertions(+) > [...] > diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter > index 240ed06..a8a4d0e 100644 > --- a/tests/qemu-iotests/common.filter > +++ b/tests/qemu-iotests/common.filter > @@ -123,6 +123,7 @@ _filter_img_info() > -e "s#$TEST_DIR#TEST_DIR#g" \ > -e "s#$IMGFMT#IMGFMT#g" \ > -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \ > + -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \ > -e "/encrypted: yes/d" \ > -e "/cluster_size: [0-9]\\+/d" \ > -e "/table_size: [0-9]\\+/d" \ > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc > index 3213765..06a3164 100644 > --- a/tests/qemu-iotests/common.rc > +++ b/tests/qemu-iotests/common.rc > @@ -89,6 +89,9 @@ else > TEST_IMG=$TEST_DIR/t.$IMGFMT > elif [ "$IMGPROTO" = "archipelago" ]; then > TEST_IMG="archipelago:at.$IMGFMT" > + elif [ "$IMGPROTO" = "vxhs" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > + TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT" > else > TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT > fi > @@ -175,6 +178,12 @@ _make_test_img() > eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT $TEST_IMG_FILE &" > sleep 1 # FIXME: qemu-nbd needs to be listening before we continue > fi > + > + # Start QNIO server on image directory for vxhs protocol > + if [ $IMGPROTO = "vxhs" ]; then > + eval "$QEMU_VXHS -d $TEST_DIR &" So I spoke too soon about tests passing, but it is not really your fault :) After rebasing to master, there is a new test 174 that now hangs (and hangs for nbd, as well). This is because the test is piping the results of _make_test_image to sed to filter it. This line should redirect stdout to /dev/null, so that the pipe does not need to wait until process completion: eval "$QEMU_VXHS -d $TEST_DIR > /dev/null &"
On Tue, Feb 14, 2017 at 10:12 AM, Jeff Cody <jcody@redhat.com> wrote: > On Mon, Nov 07, 2016 at 04:59:45PM -0800, Ashish Mittal wrote: >> These changes use a vxhs test server that is a part of the following >> repository: >> https://github.com/MittalAshish/libqnio.git >> >> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> >> --- >> v6 changelog: >> (1) Added iotests for VxHS block device. >> >> tests/qemu-iotests/common | 6 ++++++ >> tests/qemu-iotests/common.config | 13 +++++++++++++ >> tests/qemu-iotests/common.filter | 1 + >> tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ >> 4 files changed, 39 insertions(+) >> > > [...] > >> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter >> index 240ed06..a8a4d0e 100644 >> --- a/tests/qemu-iotests/common.filter >> +++ b/tests/qemu-iotests/common.filter >> @@ -123,6 +123,7 @@ _filter_img_info() >> -e "s#$TEST_DIR#TEST_DIR#g" \ >> -e "s#$IMGFMT#IMGFMT#g" \ >> -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \ >> + -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \ >> -e "/encrypted: yes/d" \ >> -e "/cluster_size: [0-9]\\+/d" \ >> -e "/table_size: [0-9]\\+/d" \ >> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc >> index 3213765..06a3164 100644 >> --- a/tests/qemu-iotests/common.rc >> +++ b/tests/qemu-iotests/common.rc >> @@ -89,6 +89,9 @@ else >> TEST_IMG=$TEST_DIR/t.$IMGFMT >> elif [ "$IMGPROTO" = "archipelago" ]; then >> TEST_IMG="archipelago:at.$IMGFMT" >> + elif [ "$IMGPROTO" = "vxhs" ]; then >> + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT >> + TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT" >> else >> TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT >> fi >> @@ -175,6 +178,12 @@ _make_test_img() >> eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT $TEST_IMG_FILE &" >> sleep 1 # FIXME: qemu-nbd needs to be listening before we continue >> fi >> + >> + # Start QNIO server on image directory for vxhs protocol >> + if [ $IMGPROTO = "vxhs" ]; then >> + eval "$QEMU_VXHS -d $TEST_DIR &" > > So I spoke too soon about tests passing, but it is not really your fault :) > > After rebasing to master, there is a new test 174 that now hangs (and hangs > for nbd, as well). This is because the test is piping the results of > _make_test_image to sed to filter it. > > This line should redirect stdout to /dev/null, so that the pipe does not > need to wait until process completion: > > eval "$QEMU_VXHS -d $TEST_DIR > /dev/null &" > > Will make this change in the next series. Thanks again!
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index d60ea2c..41430d8 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -158,6 +158,7 @@ check options -nfs test nfs -archipelago test archipelago -luks test luks + -vxhs test vxhs -xdiff graphical mode diff -nocache use O_DIRECT on backing file -misalign misalign memory allocations @@ -261,6 +262,11 @@ testlist options xpand=false ;; + -vxhs) + IMGPROTO=vxhs + xpand=false + ;; + -ssh) IMGPROTO=ssh xpand=false diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index f6384fb..c7a80c0 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -105,6 +105,10 @@ if [ -z "$QEMU_NBD_PROG" ]; then export QEMU_NBD_PROG="`set_prog_path qemu-nbd`" fi +if [ -z "$QEMU_VXHS_PROG" ]; then + export QEMU_VXHS_PROG="`set_prog_path qnio_server /usr/local/bin`" +fi + _qemu_wrapper() { ( @@ -156,10 +160,19 @@ _qemu_nbd_wrapper() ) } +_qemu_vxhs_wrapper() +{ + ( + echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid" + exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@" + ) +} + export QEMU=_qemu_wrapper export QEMU_IMG=_qemu_img_wrapper export QEMU_IO=_qemu_io_wrapper export QEMU_NBD=_qemu_nbd_wrapper +export QEMU_VXHS=_qemu_vxhs_wrapper QEMU_IMG_EXTRA_ARGS= if [ "$IMGOPTSSYNTAX" = "true" ]; then diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 240ed06..a8a4d0e 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -123,6 +123,7 @@ _filter_img_info() -e "s#$TEST_DIR#TEST_DIR#g" \ -e "s#$IMGFMT#IMGFMT#g" \ -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \ + -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \ -e "/encrypted: yes/d" \ -e "/cluster_size: [0-9]\\+/d" \ -e "/table_size: [0-9]\\+/d" \ diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 3213765..06a3164 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -89,6 +89,9 @@ else TEST_IMG=$TEST_DIR/t.$IMGFMT elif [ "$IMGPROTO" = "archipelago" ]; then TEST_IMG="archipelago:at.$IMGFMT" + elif [ "$IMGPROTO" = "vxhs" ]; then + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT + TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT" else TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT fi @@ -175,6 +178,12 @@ _make_test_img() eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT $TEST_IMG_FILE &" sleep 1 # FIXME: qemu-nbd needs to be listening before we continue fi + + # Start QNIO server on image directory for vxhs protocol + if [ $IMGPROTO = "vxhs" ]; then + eval "$QEMU_VXHS -d $TEST_DIR &" + sleep 1 # Wait for server to come up. + fi } _rm_test_img() @@ -201,6 +210,16 @@ _cleanup_test_img() fi rm -f "$TEST_IMG_FILE" ;; + vxhs) + if [ -f "${TEST_DIR}/qemu-vxhs.pid" ]; then + local QEMU_VXHS_PID + read QEMU_VXHS_PID < "${TEST_DIR}/qemu-vxhs.pid" + kill ${QEMU_VXHS_PID} >/dev/null 2>&1 + rm -f "${TEST_DIR}/qemu-vxhs.pid" + fi + rm -f "$TEST_IMG_FILE" + ;; + file) _rm_test_img "$TEST_DIR/t.$IMGFMT" _rm_test_img "$TEST_DIR/t.$IMGFMT.orig"
These changes use a vxhs test server that is a part of the following repository: https://github.com/MittalAshish/libqnio.git Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com> --- v6 changelog: (1) Added iotests for VxHS block device. tests/qemu-iotests/common | 6 ++++++ tests/qemu-iotests/common.config | 13 +++++++++++++ tests/qemu-iotests/common.filter | 1 + tests/qemu-iotests/common.rc | 19 +++++++++++++++++++ 4 files changed, 39 insertions(+)