| Message ID | CAAo6VWPhSwfqVGE4Ba08F6Kj35k--eAnSEdR0qHo-cP2Fi95=Q@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 11, 2016 at 12:56:06AM -0700, ashish mittal wrote: > Checked in a test server to libqnio that allows to open, read, write > to a vxhs vdisk using the qemu-io binary. > > Steps to run the test server: > (1) Touch a file in /tmp with the base-name of the vdisk ID to be opened. > touch /tmp/\{98f48eef-b62f-46ee-b6e3-ad48ffd9ad0a\} > (2) Start the test server. It listens on port 9999 by default. > /path/to/qnio_server > (3) In another terminal, start the qemu-io binary and open the vdisk > qemu-io> open vxhs://127.0.0.1:9999/%7B98f48eef-b62f-46ee-b6e3-ad48ffd9ad0a%7D > (4) Now you can write and read data from the vdisk that is backed by a file. > qemu-io> writev -P 81 0 1k > qemu-io> read -v 0 1k > > Following change would be needed to my last patch to allow opening of the vdisk: > $ git diff > diff --git a/block/vxhs.c b/block/vxhs.c > index 90a4343..d849a9b 100644 > --- a/block/vxhs.c > +++ b/block/vxhs.c > @@ -1215,7 +1215,7 @@ static int vxhs_qemu_init(QDict *options, > BDRVVXHSState *s, > } > > ret = vxhs_qnio_iio_open(cfd, of_vsa_addr, rfd, file_name); > - if (!ret) { > + if (ret) { > error_setg(&local_err, "Failed qnio_iio_open"); > ret = -EIO; > } > > Will work on the qemu-iotests test suite next. > > Regards, > Ashish > Hi Ashish, I wanted to check and see how the qemu-iotest suite was coming along. A couple of things on the test server: It is hardcoded to only accept 4MB images. Is it possible to relax that restriction (is a restriction even needed)? Also, the test server is hardcoded to use the path "/tmp". This needs to be passed in as an argument, as qemu-iotests will want to run everything from its scratch test directory. A suggestion: it would also be a good idea, if possible, to enable all the supported ioctl commands in the test server. Jeff
Hi Jeff, Please see my comments inline. Thanks, Ketan. On 10/18/16, 12:10 PM, "Jeff Cody" <jcody@redhat.com> wrote: >On Tue, Oct 11, 2016 at 12:56:06AM -0700, ashish mittal wrote: >> Checked in a test server to libqnio that allows to open, read, write >> to a vxhs vdisk using the qemu-io binary. >> >> Steps to run the test server: >> (1) Touch a file in /tmp with the base-name of the vdisk ID to be opened. >> touch /tmp/\{98f48eef-b62f-46ee-b6e3-ad48ffd9ad0a\} >> (2) Start the test server. It listens on port 9999 by default. >> /path/to/qnio_server >> (3) In another terminal, start the qemu-io binary and open the vdisk >> qemu-io> open vxhs://127.0.0.1:9999/%7B98f48eef-b62f-46ee-b6e3-ad48ffd9ad0a%7D >> (4) Now you can write and read data from the vdisk that is backed by a file. >> qemu-io> writev -P 81 0 1k >> qemu-io> read -v 0 1k >> >> Following change would be needed to my last patch to allow opening of the vdisk: >> $ git diff >> diff --git a/block/vxhs.c b/block/vxhs.c >> index 90a4343..d849a9b 100644 >> --- a/block/vxhs.c >> +++ b/block/vxhs.c >> @@ -1215,7 +1215,7 @@ static int vxhs_qemu_init(QDict *options, >> BDRVVXHSState *s, >> } >> >> ret = vxhs_qnio_iio_open(cfd, of_vsa_addr, rfd, file_name); >> - if (!ret) { >> + if (ret) { >> error_setg(&local_err, "Failed qnio_iio_open"); >> ret = -EIO; >> } >> >> Will work on the qemu-iotests test suite next. >> >> Regards, >> Ashish >> > >Hi Ashish, > >I wanted to check and see how the qemu-iotest suite was coming along. > >A couple of things on the test server: > >It is hardcoded to only accept 4MB images. Is it possible to relax that >restriction (is a restriction even needed)? We are removing this restriction from the QEMU driver and qnio. > >Also, the test server is hardcoded to use the path "/tmp". This needs to >be passed in as an argument, as qemu-iotests will want to run everything >from its scratch test directory. This is being worked upon and will be part of the next checkin. > >A suggestion: it would also be a good idea, if possible, to enable all the >supported ioctl commands in the test server. We will look into this. > >Jeff
diff --git a/block/vxhs.c b/block/vxhs.c index 90a4343..d849a9b 100644 --- a/block/vxhs.c +++ b/block/vxhs.c @@ -1215,7 +1215,7 @@ static int vxhs_qemu_init(QDict *options, BDRVVXHSState *s, } ret = vxhs_qnio_iio_open(cfd, of_vsa_addr, rfd, file_name); - if (!ret) { + if (ret) { error_setg(&local_err, "Failed qnio_iio_open"); ret = -EIO;
Checked in a test server to libqnio that allows to open, read, write to a vxhs vdisk using the qemu-io binary. Steps to run the test server: (1) Touch a file in /tmp with the base-name of the vdisk ID to be opened. touch /tmp/\{98f48eef-b62f-46ee-b6e3-ad48ffd9ad0a\} (2) Start the test server. It listens on port 9999 by default. /path/to/qnio_server (3) In another terminal, start the qemu-io binary and open the vdisk qemu-io> open vxhs://127.0.0.1:9999/%7B98f48eef-b62f-46ee-b6e3-ad48ffd9ad0a%7D (4) Now you can write and read data from the vdisk that is backed by a file. qemu-io> writev -P 81 0 1k qemu-io> read -v 0 1k Following change would be needed to my last patch to allow opening of the vdisk: $ git diff } Will work on the qemu-iotests test suite next. Regards, Ashish On Tue, Oct 4, 2016 at 9:02 PM, Jeff Cody <jcody@redhat.com> wrote: > On Wed, Sep 28, 2016 at 12:13:32PM +0100, Stefan Hajnoczi wrote: >> On Tue, Sep 27, 2016 at 09:09:49PM -0700, Ashish Mittal wrote: > > [...] > >> > +/* >> > + * This is called by QEMU when a flush gets triggered from within >> > + * a guest at the block layer, either for IDE or SCSI disks. >> > + */ >> > +static int vxhs_co_flush(BlockDriverState *bs) >> >> This is called from coroutine context, please add the coroutine_fn >> function attribute to document this. >> >> > +{ >> > + BDRVVXHSState *s = bs->opaque; >> > + int64_t size = 0; >> > + int ret = 0; >> > + >> > + /* >> > + * VDISK_AIO_FLUSH ioctl is a no-op at present and will >> > + * always return success. This could change in the future. >> > + */ >> > + ret = vxhs_qnio_iio_ioctl(s->qnio_ctx, >> > + s->vdisk_hostinfo[s->vdisk_cur_host_idx].vdisk_rfd, >> > + VDISK_AIO_FLUSH, &size, NULL, IIO_FLAG_SYNC); >> >> This function is not allowed to block. It cannot do a synchronous >> flush. This line is misleading because the constant is called >> VDISK_AIO_FLUSH, but looking at the library code I see it's actually a >> synchronous call that ends up in a loop that sleeps (!) waiting for the >> response. >> >> Please do an async flush and qemu_coroutine_yield() to return >> control to QEMU's event loop. When the flush completes you can >> qemu_coroutine_enter() again to return from this function. >> >> > + >> > + if (ret < 0) { >> > + trace_vxhs_co_flush(s->vdisk_guid, ret, errno); >> > + vxhs_close(bs); >> >> This looks unsafe. Won't it cause double close() calls for s->fds[] >> when bdrv_close() is called later? >> > > Calling the close on a failed flush is a good idea, in my opinion. However, > to make it safe, bs->drv MUST be set to NULL after the call to vxhs_close(). > That will prevent double free / close calls, and also fail out new I/O. > (This is actually what the gluster driver does, for instance). > > Jeff > > >