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

2019-08-25 Thread Sam Eiderman via Qemu-devel 
> Only scsi-hd has the lchs properties, though, so what’s the purpose of
> defining the unrealize function for all other classes?
>
> Max

- shmuel.eider...@oracle.com
+ sam...@google.com

The only purpose is to already have them mapped to the correct existing
function, in case it will be used later on.
I can resubmit without the unrealize for the other classes, WDYT?

Sam

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

2019-08-13 Thread Max Reitz 
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 
> Reviewed-by: Arbel Moshe 
> Signed-off-by: Sam Eiderman 
> ---
>  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)->lcyls,
> + (>conf)->lheads,
> + (>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



signature.asc
Description: OpenPGP digital signature

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

2019-06-26 Thread Sam Eiderman 
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 
Reviewed-by: Arbel Moshe 
Signed-off-by: Sam Eiderman 
---
 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)->lcyls,
+ (>conf)->lheads,
+ (>conf)->lsecs);
 }
 
 static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
@@ -1189,6 +1194,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev, 
Error **errp)
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOBlock *s = VIRTIO_BLK(dev);
 
+del_boot_device_lchs(dev, "/disk@0,0");
 virtio_blk_data_plane_destroy(s->dataplane);
 s->dataplane = NULL;
 qemu_del_vm_change_state_handler(s->change);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 9cae3205df..07f429d5e3 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -215,6 +215,11 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
kind, Error **errp)
 
 add_boot_device_path(dev->conf.bootindex, >qdev,
  dev->unit ? "/disk@1" : "/disk@0");
+
+add_boot_device_lchs(>qdev, dev->unit ? "/disk@1" : "/disk@0",
+ (>conf)->lcyls,
+ (>conf)->lheads,
+ (>conf)->lsecs);
 }
 
 static void ide_dev_get_bootindex(Object *obj, Visitor *v, const char *name,
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
@@ -2390,6 +2390,16 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
 
 blk_iostatus_enable(s->qdev.conf.blk);
+
+add_boot_device_lchs(>qdev, NULL,
+ (>conf)->lcyls,
+ (>conf)->lheads,
+ (>conf)->lsecs);
+}
+
+static void scsi_unrealize(SCSIDevice *dev, Error **errp)
+{
+del_boot_device_lchs(>qdev, NULL);
 }
 
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
@@ -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";
-- 
2.13.3