| Message ID | 1345170981-7738-3-git-send-email-ronniesahlberg@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Aug 17, 2012 at 2:36 AM, Ronnie Sahlberg <ronniesahlberg@gmail.com> wrote: > There is no bdrv_* API for the commands for burning a blank MMC disk > so when iSCSI LUNs are specified and the LUN is a MMC device with > 0 available blocks. This is a blank disk so force scsi generic. > > This allows the guest to talk directly to the target to burn data on > the disk. > > Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> > --- > block/iscsi.c | 13 +++++++++++-- > 1 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index fb420ea..ca53afa 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1017,10 +1017,19 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags) > /* Medium changer or tape. We dont have any emulation for this so this must > * be sg ioctl compatible. We force it to be sg, otherwise qemu will try > * to read from the device to guess the image format. > + * MMC device with no blocks contain a blank disk so force them to use sg > + * too. > */ > - if (iscsilun->type == TYPE_MEDIUM_CHANGER || > - iscsilun->type == TYPE_TAPE) { > + switch (iscsilun->type) { > + case TYPE_ROM: > + if (iscsilun->num_blocks > 0) { > + break; > + } Please don't fall through without a comment. > + case TYPE_MEDIUM_CHANGER: > + case TYPE_TAPE: > bs->sg = 1; Also here, but I'd add 'break' instead. > + default: > + break; > } > > ret = 0; > -- > 1.7.3.1 > >
Il 17/08/2012 04:36, Ronnie Sahlberg ha scritto: > There is no bdrv_* API for the commands for burning a blank MMC disk > so when iSCSI LUNs are specified and the LUN is a MMC device with > 0 available blocks. This is a blank disk so force scsi generic. > > This allows the guest to talk directly to the target to burn data on > the disk. > > Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> What happens without the patch? It's ok that scsi-{hd,cd} does not work, but do scsi-{block,generic} work without the patch? Paolo
On Sun, Aug 19, 2012 at 7:58 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 17/08/2012 04:36, Ronnie Sahlberg ha scritto: >> There is no bdrv_* API for the commands for burning a blank MMC disk >> so when iSCSI LUNs are specified and the LUN is a MMC device with >> 0 available blocks. This is a blank disk so force scsi generic. >> >> This allows the guest to talk directly to the target to burn data on >> the disk. >> >> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> > > What happens without the patch? It's ok that scsi-{hd,cd} does not > work, but do scsi-{block,generic} work without the patch? > Neither of them work, basically because in block.c:find_image_format() if bs->sg is not set in if (bs->sg || !bdrv_is_inserted(bs)) { then we continue to ret = bdrv_pread(bs, 0, buf, sizeof(buf)); which fails with an error. So this patch is basically to prevent find_image_format() from trying to read from a blank disk. Maybe there is a better way to do this? regards ronnie sahlberg
Ah, This patch only affects the case when there is a blank / empty disk loaded. It has no effect on when real *.iso images are loaded and the disk contains data. The use case to be able to "burn" to an iscsi cdrom is probably not very urgent, so maybe it is best to delay this until post 1.2 regards ronnie sahlberg On Sun, Aug 19, 2012 at 8:02 AM, ronnie sahlberg <ronniesahlberg@gmail.com> wrote: > On Sun, Aug 19, 2012 at 7:58 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Il 17/08/2012 04:36, Ronnie Sahlberg ha scritto: >>> There is no bdrv_* API for the commands for burning a blank MMC disk >>> so when iSCSI LUNs are specified and the LUN is a MMC device with >>> 0 available blocks. This is a blank disk so force scsi generic. >>> >>> This allows the guest to talk directly to the target to burn data on >>> the disk. >>> >>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> >> What happens without the patch? It's ok that scsi-{hd,cd} does not >> work, but do scsi-{block,generic} work without the patch? >> > > Neither of them work, basically because in > block.c:find_image_format() > > if bs->sg is not set in > > if (bs->sg || !bdrv_is_inserted(bs)) { > > then we continue to > > ret = bdrv_pread(bs, 0, buf, sizeof(buf)); > > which fails with an error. > So this patch is basically to prevent find_image_format() from trying > to read from a blank disk. > > Maybe there is a better way to do this? > > > > regards > ronnie sahlberg
Il 19/08/2012 00:02, ronnie sahlberg ha scritto: > Neither of them work, basically because in > block.c:find_image_format() > > if bs->sg is not set in > > if (bs->sg || !bdrv_is_inserted(bs)) { > > then we continue to > > ret = bdrv_pread(bs, 0, buf, sizeof(buf)); > > which fails with an error. > So this patch is basically to prevent find_image_format() from trying > to read from a blank disk. > > Maybe there is a better way to do this? Yeah, I think in this case find_image_format should just use raw. Paolo
On Sun, Aug 19, 2012 at 8:16 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 19/08/2012 00:02, ronnie sahlberg ha scritto: >> Neither of them work, basically because in >> block.c:find_image_format() >> >> if bs->sg is not set in >> >> if (bs->sg || !bdrv_is_inserted(bs)) { >> >> then we continue to >> >> ret = bdrv_pread(bs, 0, buf, sizeof(buf)); >> >> which fails with an error. >> So this patch is basically to prevent find_image_format() from trying >> to read from a blank disk. >> >> Maybe there is a better way to do this? > > Yeah, I think in this case find_image_format should just use raw. Ok, so that is basically what the patch does. It forces bs->sg==true so that we pick "raw" right there instead of trying to read from the device. So you are happy with the patch ? regards ronnie sahlberg
Il 19/08/2012 00:02, ronnie sahlberg ha scritto: > On Sun, Aug 19, 2012 at 7:58 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Il 17/08/2012 04:36, Ronnie Sahlberg ha scritto: >>> There is no bdrv_* API for the commands for burning a blank MMC disk >>> so when iSCSI LUNs are specified and the LUN is a MMC device with >>> 0 available blocks. This is a blank disk so force scsi generic. >>> >>> This allows the guest to talk directly to the target to burn data on >>> the disk. >>> >>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> >> What happens without the patch? It's ok that scsi-{hd,cd} does not >> work, but do scsi-{block,generic} work without the patch? >> > > Neither of them work, basically because in > block.c:find_image_format() > > if bs->sg is not set in > > if (bs->sg || !bdrv_is_inserted(bs)) { > > then we continue to > > ret = bdrv_pread(bs, 0, buf, sizeof(buf)); > > which fails with an error. > So this patch is basically to prevent find_image_format() from trying > to read from a blank disk. > > Maybe there is a better way to do this? Ah, BTW does using -drive ...,format=raw work, or does it hiccup later again on something else? Paolo
Il 19/08/2012 00:19, ronnie sahlberg ha scritto: >> > Yeah, I think in this case find_image_format should just use raw. > Ok, so that is basically what the patch does. It forces bs->sg==true > so that we pick "raw" right there instead of trying to read from the > device. > > So you are happy with the patch ? No, the solution should be the same that allows "touch ff + qemu-kvm -hda ff" to work. This is implemented here: if (ret >= 0 && ret < aiocb->aio_nbytes && aiocb->common.bs->growable) { /* A short read means that we have reached EOF. Pad the buffer * with zeros for bytes after EOF. */ iov_memset(aiocb->aio_iov, aiocb->aio_niov, ret, 0, aiocb->aio_nbytes - ret); ret = aiocb->aio_nbytes; } and block/iscsi.c should do the same. Paolo
On Sun, Aug 19, 2012 at 8:20 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 19/08/2012 00:02, ronnie sahlberg ha scritto: >> On Sun, Aug 19, 2012 at 7:58 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> Il 17/08/2012 04:36, Ronnie Sahlberg ha scritto: >>>> There is no bdrv_* API for the commands for burning a blank MMC disk >>>> so when iSCSI LUNs are specified and the LUN is a MMC device with >>>> 0 available blocks. This is a blank disk so force scsi generic. >>>> >>>> This allows the guest to talk directly to the target to burn data on >>>> the disk. >>>> >>>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> >>> >>> What happens without the patch? It's ok that scsi-{hd,cd} does not >>> work, but do scsi-{block,generic} work without the patch? >>> >> >> Neither of them work, basically because in >> block.c:find_image_format() >> >> if bs->sg is not set in >> >> if (bs->sg || !bdrv_is_inserted(bs)) { >> >> then we continue to >> >> ret = bdrv_pread(bs, 0, buf, sizeof(buf)); >> >> which fails with an error. >> So this patch is basically to prevent find_image_format() from trying >> to read from a blank disk. >> >> Maybe there is a better way to do this? > > Ah, BTW does using -drive ...,format=raw work, or does it hiccup later > again on something else? > format=raw works ! That then begs the question if would it be possible to force format=raw always for iscsi devices? A iscsi device as far as I can see would always just be a raw block device and there would never be a "header" on such devices so maybe that would be a solution? regards ronnie sahlberg
Il 19/08/2012 01:44, ronnie sahlberg ha scritto: > format=raw works ! > > That then begs the question if would it be possible to force > format=raw always for iscsi devices? > > A iscsi device as far as I can see would always just be a raw block > device and there would never be a "header" on such devices > so maybe that would be a solution? While it sounds weird, it is perfectly possible to host a qcow2 image on an iSCSI LUN. So the solution is to make iscsi behave the same as raw-posix and raw-win32 (since they all sit at the same level). Paolo
diff --git a/block/iscsi.c b/block/iscsi.c index fb420ea..ca53afa 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1017,10 +1017,19 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags) /* Medium changer or tape. We dont have any emulation for this so this must * be sg ioctl compatible. We force it to be sg, otherwise qemu will try * to read from the device to guess the image format. + * MMC device with no blocks contain a blank disk so force them to use sg + * too. */ - if (iscsilun->type == TYPE_MEDIUM_CHANGER || - iscsilun->type == TYPE_TAPE) { + switch (iscsilun->type) { + case TYPE_ROM: + if (iscsilun->num_blocks > 0) { + break; + } + case TYPE_MEDIUM_CHANGER: + case TYPE_TAPE: bs->sg = 1; + default: + break; } ret = 0;
There is no bdrv_* API for the commands for burning a blank MMC disk so when iSCSI LUNs are specified and the LUN is a MMC device with 0 available blocks. This is a blank disk so force scsi generic. This allows the guest to talk directly to the target to burn data on the disk. Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> --- block/iscsi.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-)