| Message ID | 1468504183-20180-2-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
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?
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.
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
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); }
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(-)