[Qemu-devel] [PATCH 0/2] block: Create empty drives without explicit BB

[Qemu-devel] [PATCH 0/2] block: Create empty drives without explicit BB

[Kevin Wolf]

This series allows creating empty IDE and SCSI CD-ROM drives by leaving
out the drive=... option. At the moment, such drives are still
relatively useless because you can't insert a medium at runtime yet, but
we'll change the QMP commands soon to accept qdev device names instead
of BlockBackend names.

Kevin Wolf (2):
  ide: ide-cd without drive property for empty drive
  scsi: scsi-cd without drive property for empty drive

 hw/ide/qdev.c       | 20 +++++++++++++++-----
 hw/scsi/scsi-disk.c |  5 +++++
 2 files changed, 20 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive

[Kevin Wolf]

This allows to create an empty ide-cd device without manually creating a
BlockBackend.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/qdev.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 67c76bf..2eb055a 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -75,10 +75,6 @@ static int ide_qdev_init(DeviceState *qdev)
     IDEDeviceClass *dc = IDE_DEVICE_GET_CLASS(dev);
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev->parent_bus);
 
-    if (!dev->conf.blk) {
-        error_report("No drive specified");
-        goto err;
-    }
     if (dev->unit == -1) {
         dev->unit = bus->master ? 1 : 0;
     }
@@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     IDEState *s = bus->ifs + dev->unit;
     Error *err = NULL;
 
+    if (!dev->conf.blk) {
+        if (kind != IDE_CD) {
+            error_report("No drive specified");
+            return -1;
+        } else {
+            /* Anonymous BlockBackend for an empty drive */
+            dev->conf.blk = blk_new();
+        }
+    }
+
     if (dev->conf.discard_granularity == -1) {
         dev->conf.discard_granularity = 512;
     } else if (dev->conf.discard_granularity &&
@@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
 
 static int ide_drive_initfn(IDEDevice *dev)
 {
-    DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
+    DriveInfo *dinfo = NULL;
+
+    if (dev->conf.blk) {
+        dinfo = blk_legacy_dinfo(dev->conf.blk);
+    }
 
     return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] scsi: scsi-cd without drive property for empty drive

[Kevin Wolf]

This allows to create an empty scsi-cd device without manually creating
a BlockBackend.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi/scsi-disk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 8dbfc10..b4e71ff 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2359,6 +2359,11 @@ static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
 static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+
+    if (!dev->conf.blk) {
+        dev->conf.blk = blk_new();
+    }
+
     s->qdev.blocksize = 2048;
     s->qdev.type = TYPE_ROM;
     s->features |= 1 << SCSI_DISK_F_REMOVABLE;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On 07/14/2016 07:49 AM, Kevin Wolf wrote:
> This allows to create an empty ide-cd device without manually creating a
> BlockBackend.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  hw/ide/qdev.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)

> @@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
>      IDEState *s = bus->ifs + dev->unit;
>      Error *err = NULL;
>  
> +    if (!dev->conf.blk) {
> +        if (kind != IDE_CD) {
> +            error_report("No drive specified");
> +            return -1;
> +        } else {
> +            /* Anonymous BlockBackend for an empty drive */
> +            dev->conf.blk = blk_new();

So we either fail or dev->conf.blk is set...

> +        }
> +    }
> +
>      if (dev->conf.discard_granularity == -1) {
>          dev->conf.discard_granularity = 512;
>      } else if (dev->conf.discard_granularity &&
> @@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
>  
>  static int ide_drive_initfn(IDEDevice *dev)
>  {
> -    DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
> +    DriveInfo *dinfo = NULL;
> +
> +    if (dev->conf.blk) {
> +        dinfo = blk_legacy_dinfo(dev->conf.blk);
> +    }
>  
>      return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);

...yet, this claims dev->conf.blk can be NULL.  What am I missing?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/2] scsi: scsi-cd without drive property for empty drive

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On 07/14/2016 07:49 AM, Kevin Wolf wrote:
> This allows to create an empty scsi-cd device without manually creating

Grammar might sound better as:

This allows the creation of an empty

or

This allows a user to create an empty

> a BlockBackend.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  hw/scsi/scsi-disk.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 8dbfc10..b4e71ff 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2359,6 +2359,11 @@ static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
>  static void scsi_cd_realize(SCSIDevice *dev, Error **errp)
>  {
>      SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> +
> +    if (!dev->conf.blk) {
> +        dev->conf.blk = blk_new();
> +    }
> +

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1686 bytes --]

Am 14.07.2016 um 21:48 hat Eric Blake geschrieben:
> On 07/14/2016 07:49 AM, Kevin Wolf wrote:
> > This allows to create an empty ide-cd device without manually creating a
> > BlockBackend.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  hw/ide/qdev.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> > @@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
> >      IDEState *s = bus->ifs + dev->unit;
> >      Error *err = NULL;
> >  
> > +    if (!dev->conf.blk) {
> > +        if (kind != IDE_CD) {
> > +            error_report("No drive specified");
> > +            return -1;
> > +        } else {
> > +            /* Anonymous BlockBackend for an empty drive */
> > +            dev->conf.blk = blk_new();
> 
> So we either fail or dev->conf.blk is set...
> 
> > +        }
> > +    }
> > +
> >      if (dev->conf.discard_granularity == -1) {
> >          dev->conf.discard_granularity = 512;
> >      } else if (dev->conf.discard_granularity &&
> > @@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
> >  
> >  static int ide_drive_initfn(IDEDevice *dev)
> >  {
> > -    DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
> > +    DriveInfo *dinfo = NULL;
> > +
> > +    if (dev->conf.blk) {
> > +        dinfo = blk_legacy_dinfo(dev->conf.blk);
> > +    }
> >  
> >      return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
> 
> ...yet, this claims dev->conf.blk can be NULL.  What am I missing?

That ide_drive_initfn() is the outer function and runs first, before we
handle the case in ide_dev_initfn()?

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

On 08/01/2016 01:47 PM, Kevin Wolf wrote:
> Am 14.07.2016 um 21:48 hat Eric Blake geschrieben:
>> On 07/14/2016 07:49 AM, Kevin Wolf wrote:
>>> This allows to create an empty ide-cd device without manually creating a
>>> BlockBackend.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>  hw/ide/qdev.c | 20 +++++++++++++++-----
>>>  1 file changed, 15 insertions(+), 5 deletions(-)
>>
>>> @@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
>>>      IDEState *s = bus->ifs + dev->unit;
>>>      Error *err = NULL;
>>>  
>>> +    if (!dev->conf.blk) {
>>> +        if (kind != IDE_CD) {
>>> +            error_report("No drive specified");
>>> +            return -1;
>>> +        } else {
>>> +            /* Anonymous BlockBackend for an empty drive */
>>> +            dev->conf.blk = blk_new();
>>
>> So we either fail or dev->conf.blk is set...
>>
>>> +        }
>>> +    }
>>> +
>>>      if (dev->conf.discard_granularity == -1) {
>>>          dev->conf.discard_granularity = 512;
>>>      } else if (dev->conf.discard_granularity &&
>>> @@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
>>>  
>>>  static int ide_drive_initfn(IDEDevice *dev)
>>>  {
>>> -    DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
>>> +    DriveInfo *dinfo = NULL;
>>> +
>>> +    if (dev->conf.blk) {
>>> +        dinfo = blk_legacy_dinfo(dev->conf.blk);
>>> +    }
>>>  
>>>      return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
>>
>> ...yet, this claims dev->conf.blk can be NULL.  What am I missing?
> 
> That ide_drive_initfn() is the outer function and runs first, before we
> handle the case in ide_dev_initfn()?

Ah, I think I glazed over the difference between 'dev' and 'drive', and
was thinking 'both of these are initfn, what's different?'.  I think you
are okay, after all.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

Am 04.08.2016 um 18:03 hat Eric Blake geschrieben:
> On 08/01/2016 01:47 PM, Kevin Wolf wrote:
> > Am 14.07.2016 um 21:48 hat Eric Blake geschrieben:
> >> On 07/14/2016 07:49 AM, Kevin Wolf wrote:
> >>> This allows to create an empty ide-cd device without manually creating a
> >>> BlockBackend.
> >>>
> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >>> ---
> >>>  hw/ide/qdev.c | 20 +++++++++++++++-----
> >>>  1 file changed, 15 insertions(+), 5 deletions(-)
> >>
> >>> @@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
> >>>      IDEState *s = bus->ifs + dev->unit;
> >>>      Error *err = NULL;
> >>>  
> >>> +    if (!dev->conf.blk) {
> >>> +        if (kind != IDE_CD) {
> >>> +            error_report("No drive specified");
> >>> +            return -1;
> >>> +        } else {
> >>> +            /* Anonymous BlockBackend for an empty drive */
> >>> +            dev->conf.blk = blk_new();
> >>
> >> So we either fail or dev->conf.blk is set...
> >>
> >>> +        }
> >>> +    }
> >>> +
> >>>      if (dev->conf.discard_granularity == -1) {
> >>>          dev->conf.discard_granularity = 512;
> >>>      } else if (dev->conf.discard_granularity &&
> >>> @@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
> >>>  
> >>>  static int ide_drive_initfn(IDEDevice *dev)
> >>>  {
> >>> -    DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
> >>> +    DriveInfo *dinfo = NULL;
> >>> +
> >>> +    if (dev->conf.blk) {
> >>> +        dinfo = blk_legacy_dinfo(dev->conf.blk);
> >>> +    }
> >>>  
> >>>      return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
> >>
> >> ...yet, this claims dev->conf.blk can be NULL.  What am I missing?
> > 
> > That ide_drive_initfn() is the outer function and runs first, before we
> > handle the case in ide_dev_initfn()?
> 
> Ah, I think I glazed over the difference between 'dev' and 'drive', and
> was thinking 'both of these are initfn, what's different?'.  I think you
> are okay, after all.

I noted this down as an Acked-by for now. If you decide to give an actual
R-b, I'll update that.

Applied the series to block-next.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]