| Message ID | 1425061593-4411-2-git-send-email-tumanova@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes: > check conf.blk before calling blkconf_blocksizes > > Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> > --- > hw/scsi/scsi-disk.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index 2921728..df5140e 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) > static void scsi_hd_realize(SCSIDevice *dev, Error **errp) > { > SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); > - blkconf_blocksizes(&s->qdev.conf); > + if (s->qdev.conf.blk) { > + blkconf_blocksizes(&s->qdev.conf); > + } Looks suspicious on first glance, because block device model realize() methods are supposed to fail when the backend is missing. But... > s->qdev.blocksize = s->qdev.conf.logical_block_size; > s->qdev.type = TYPE_DISK; > if (!s->product) { s->product = g_strdup("QEMU HARDDISK"); } scsi_realize(&s->qdev, errp); ... scsi_realize() errors out then. Worth a comment. Or maybe call blkconf_blocksizes() only after scsi_realize(). Your choice.
On 03/02/2015 11:46 AM, Markus Armbruster wrote: > Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes: > >> check conf.blk before calling blkconf_blocksizes >> >> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> >> --- >> hw/scsi/scsi-disk.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c >> index 2921728..df5140e 100644 >> --- a/hw/scsi/scsi-disk.c >> +++ b/hw/scsi/scsi-disk.c >> @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) >> static void scsi_hd_realize(SCSIDevice *dev, Error **errp) >> { >> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); >> - blkconf_blocksizes(&s->qdev.conf); >> + if (s->qdev.conf.blk) { >> + blkconf_blocksizes(&s->qdev.conf); >> + } > > Looks suspicious on first glance, because block device model realize() > methods are supposed to fail when the backend is missing. But... > it will properly fail in scsi_realize >> s->qdev.blocksize = s->qdev.conf.logical_block_size; >> s->qdev.type = TYPE_DISK; >> if (!s->product) { > s->product = g_strdup("QEMU HARDDISK"); > } > scsi_realize(&s->qdev, errp); > > ... scsi_realize() errors out then. Worth a comment. Or maybe call > blkconf_blocksizes() only after scsi_realize(). Your choice. can't call it later. conf.logical_block_size, which blkconf_blocksizes sets it used earlier.
Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes: > On 03/02/2015 11:46 AM, Markus Armbruster wrote: >> Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes: >> >>> check conf.blk before calling blkconf_blocksizes >>> >>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> >>> --- >>> hw/scsi/scsi-disk.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c >>> index 2921728..df5140e 100644 >>> --- a/hw/scsi/scsi-disk.c >>> +++ b/hw/scsi/scsi-disk.c >>> @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) >>> static void scsi_hd_realize(SCSIDevice *dev, Error **errp) >>> { >>> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); >>> - blkconf_blocksizes(&s->qdev.conf); >>> + if (s->qdev.conf.blk) { >>> + blkconf_blocksizes(&s->qdev.conf); >>> + } >> >> Looks suspicious on first glance, because block device model realize() >> methods are supposed to fail when the backend is missing. But... >> > > it will properly fail in scsi_realize > >>> s->qdev.blocksize = s->qdev.conf.logical_block_size; >>> s->qdev.type = TYPE_DISK; >>> if (!s->product) { >> s->product = g_strdup("QEMU HARDDISK"); >> } >> scsi_realize(&s->qdev, errp); >> >> ... scsi_realize() errors out then. Worth a comment. Or maybe call >> blkconf_blocksizes() only after scsi_realize(). Your choice. > > can't call it later. conf.logical_block_size, which blkconf_blocksizes > sets it used earlier. Okay, I recommend a comment then.
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 2921728..df5140e 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2291,7 +2291,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) static void scsi_hd_realize(SCSIDevice *dev, Error **errp) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); - blkconf_blocksizes(&s->qdev.conf); + if (s->qdev.conf.blk) { + blkconf_blocksizes(&s->qdev.conf); + } s->qdev.blocksize = s->qdev.conf.logical_block_size; s->qdev.type = TYPE_DISK; if (!s->product) {
check conf.blk before calling blkconf_blocksizes Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> --- hw/scsi/scsi-disk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)