Re: [Qemu-devel] [QEMU] [PATCH v5 5/8] bootdevice: Gather LCHS from all relevant devices

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3188 bytes --]

On 26.06.19 14:39, Sam Eiderman wrote:
> Relevant devices are:
>     * ide-hd (and ide-cd, ide-drive)
>     * scsi-hd (and scsi-cd, scsi-disk, scsi-block)
>     * virtio-blk-pci
> 
> We do not call del_boot_device_lchs() for ide-* since we don't need to -
> IDE block devices do not support unplugging.
> 
> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
> Reviewed-by: Arbel Moshe <arbel.moshe@oracle.com>
> Signed-off-by: Sam Eiderman <shmuel.eiderman@oracle.com>
> ---
>  hw/block/virtio-blk.c |  6 ++++++
>  hw/ide/qdev.c         |  5 +++++
>  hw/scsi/scsi-disk.c   | 14 ++++++++++++++
>  3 files changed, 25 insertions(+)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 06e57a4d39..787bbd768a 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -1182,6 +1182,11 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
>      blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
>  
>      blk_iostatus_enable(s->blk);
> +
> +    add_boot_device_lchs(dev, "/disk@0,0",
> +                         (&conf->conf)->lcyls,
> +                         (&conf->conf)->lheads,
> +                         (&conf->conf)->lsecs);

...why not simply “conf->conf.lcyls” and so on?

[...]

> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 7b89ac798b..3451aefdea 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c

[...]

> @@ -2988,6 +2998,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void *data)
>      SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
>  
>      sc->realize      = scsi_hd_realize;
> +    sc->unrealize    = scsi_unrealize;
>      sc->alloc_req    = scsi_new_request;
>      sc->unit_attention_reported = scsi_disk_unit_attention_reported;
>      dc->desc = "virtual SCSI disk";
> @@ -3019,6 +3030,7 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void *data)
>      SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
>  
>      sc->realize      = scsi_cd_realize;
> +    sc->unrealize    = scsi_unrealize;
>      sc->alloc_req    = scsi_new_request;
>      sc->unit_attention_reported = scsi_disk_unit_attention_reported;
>      dc->desc = "virtual SCSI CD-ROM";
> @@ -3054,6 +3066,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, void *data)
>      SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
>  
>      sc->realize      = scsi_block_realize;
> +    sc->unrealize    = scsi_unrealize;
>      sc->alloc_req    = scsi_block_new_request;
>      sc->parse_cdb    = scsi_block_parse_cdb;
>      sdc->dma_readv   = scsi_block_dma_readv;
> @@ -3095,6 +3108,7 @@ static void scsi_disk_class_initfn(ObjectClass *klass, void *data)
>      SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
>  
>      sc->realize      = scsi_disk_realize;
> +    sc->unrealize    = scsi_unrealize;
>      sc->alloc_req    = scsi_new_request;
>      sc->unit_attention_reported = scsi_disk_unit_attention_reported;
>      dc->fw_name = "disk";

Only scsi-hd has the lchs properties, though, so what’s the purpose of
defining the unrealize function for all other classes?

Max


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