[PATCH v2] xen: rework pci_piix3_xen_ide_unplug

[PATCH v2] xen: rework pci_piix3_xen_ide_unplug

[Anthony PERARD via]

From: Anthony PERARD <anthony.perard@citrix.com>

This is to allow IDE disks to be unplugged when adding to QEMU via:
    -drive file=/root/disk_file,if=none,id=ide-disk0,format=raw
    -device ide-hd,drive=ide-disk0,bus=ide.0,unit=0

as the current code only works for disk added with:
    -drive file=/root/disk_file,if=ide,index=0,media=disk,format=raw

Since the code already have the IDE controller as `dev`, we don't need
to use the legacy DriveInfo to find all the drive we want to unplug.
We can simply use `blk` from the controller, as it kind of was already
assume to be the same, by setting it to NULL.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

---
v2: coding style

CC: Paul Durrant <paul@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
---
 hw/ide/piix.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index b402a936362b..b9860e35a5c4 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -164,30 +164,29 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
 int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
 {
     PCIIDEState *pci_ide;
-    DriveInfo *di;
     int i;
     IDEDevice *idedev;
+    IDEBus *idebus;
+    BlockBackend *blk;
 
     pci_ide = PCI_IDE(dev);
 
     for (i = aux ? 1 : 0; i < 4; i++) {
-        di = drive_get_by_index(IF_IDE, i);
-        if (di != NULL && !di->media_cd) {
-            BlockBackend *blk = blk_by_legacy_dinfo(di);
-            DeviceState *ds = blk_get_attached_dev(blk);
+        idebus = &pci_ide->bus[i / 2];
+        blk = idebus->ifs[i % 2].blk;
 
-            blk_drain(blk);
-            blk_flush(blk);
-
-            if (ds) {
-                blk_detach_dev(blk, ds);
-            }
-            pci_ide->bus[di->bus].ifs[di->unit].blk = NULL;
+        if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) {
             if (!(i % 2)) {
-                idedev = pci_ide->bus[di->bus].master;
+                idedev = idebus->master;
             } else {
-                idedev = pci_ide->bus[di->bus].slave;
+                idedev = idebus->slave;
             }
+
+            blk_drain(blk);
+            blk_flush(blk);
+
+            blk_detach_dev(blk, DEVICE(idedev));
+            idebus->ifs[i % 2].blk = NULL;
             idedev->conf.blk = NULL;
             monitor_remove_blk(blk);
             blk_unref(blk);
-- 
Anthony PERARD

RE: [PATCH v2] xen: rework pci_piix3_xen_ide_unplug

[Paul Durrant]

> -----Original Message-----
> From: Anthony PERARD <anthony.perard@citrix.com>
> Sent: 27 October 2020 15:41
> To: qemu-devel@nongnu.org
> Cc: Anthony PERARD <anthony.perard@citrix.com>; Paul Durrant <paul@xen.org>; Stefano Stabellini
> <sstabellini@kernel.org>; John Snow <jsnow@redhat.com>; qemu-block@nongnu.org
> Subject: [PATCH v2] xen: rework pci_piix3_xen_ide_unplug
> 
> From: Anthony PERARD <anthony.perard@citrix.com>
> 
> This is to allow IDE disks to be unplugged when adding to QEMU via:
>     -drive file=/root/disk_file,if=none,id=ide-disk0,format=raw
>     -device ide-hd,drive=ide-disk0,bus=ide.0,unit=0
> 
> as the current code only works for disk added with:
>     -drive file=/root/disk_file,if=ide,index=0,media=disk,format=raw
> 
> Since the code already have the IDE controller as `dev`, we don't need
> to use the legacy DriveInfo to find all the drive we want to unplug.
> We can simply use `blk` from the controller, as it kind of was already
> assume to be the same, by setting it to NULL.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 

Reviewed-by: Paul Durrant <paul@xen.org>

> ---
> v2: coding style
> 
> CC: Paul Durrant <paul@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> ---
>  hw/ide/piix.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index b402a936362b..b9860e35a5c4 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -164,30 +164,29 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
>  int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
>  {
>      PCIIDEState *pci_ide;
> -    DriveInfo *di;
>      int i;
>      IDEDevice *idedev;
> +    IDEBus *idebus;
> +    BlockBackend *blk;
> 
>      pci_ide = PCI_IDE(dev);
> 
>      for (i = aux ? 1 : 0; i < 4; i++) {
> -        di = drive_get_by_index(IF_IDE, i);
> -        if (di != NULL && !di->media_cd) {
> -            BlockBackend *blk = blk_by_legacy_dinfo(di);
> -            DeviceState *ds = blk_get_attached_dev(blk);
> +        idebus = &pci_ide->bus[i / 2];
> +        blk = idebus->ifs[i % 2].blk;
> 
> -            blk_drain(blk);
> -            blk_flush(blk);
> -
> -            if (ds) {
> -                blk_detach_dev(blk, ds);
> -            }
> -            pci_ide->bus[di->bus].ifs[di->unit].blk = NULL;
> +        if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) {
>              if (!(i % 2)) {
> -                idedev = pci_ide->bus[di->bus].master;
> +                idedev = idebus->master;
>              } else {
> -                idedev = pci_ide->bus[di->bus].slave;
> +                idedev = idebus->slave;
>              }
> +
> +            blk_drain(blk);
> +            blk_flush(blk);
> +
> +            blk_detach_dev(blk, DEVICE(idedev));
> +            idebus->ifs[i % 2].blk = NULL;
>              idedev->conf.blk = NULL;
>              monitor_remove_blk(blk);
>              blk_unref(blk);
> --
> Anthony PERARD

Re: [PATCH v2] xen: rework pci_piix3_xen_ide_unplug

[John Snow]

On 10/27/20 11:40 AM, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
> 
> This is to allow IDE disks to be unplugged when adding to QEMU via:
>      -drive file=/root/disk_file,if=none,id=ide-disk0,format=raw
>      -device ide-hd,drive=ide-disk0,bus=ide.0,unit=0
> 
> as the current code only works for disk added with:
>      -drive file=/root/disk_file,if=ide,index=0,media=disk,format=raw
> 
> Since the code already have the IDE controller as `dev`, we don't need
> to use the legacy DriveInfo to find all the drive we want to unplug.
> We can simply use `blk` from the controller, as it kind of was already
> assume to be the same, by setting it to NULL.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: John Snow <jsnow@redhat.com>

Do you need me to send a PR for this?

--js

> 
> ---
> v2: coding style
> 
> CC: Paul Durrant <paul@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> ---
>   hw/ide/piix.c | 27 +++++++++++++--------------
>   1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index b402a936362b..b9860e35a5c4 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -164,30 +164,29 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
>   int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
>   {
>       PCIIDEState *pci_ide;
> -    DriveInfo *di;
>       int i;
>       IDEDevice *idedev;
> +    IDEBus *idebus;
> +    BlockBackend *blk;
>   
>       pci_ide = PCI_IDE(dev);
>   
>       for (i = aux ? 1 : 0; i < 4; i++) {
> -        di = drive_get_by_index(IF_IDE, i);
> -        if (di != NULL && !di->media_cd) {
> -            BlockBackend *blk = blk_by_legacy_dinfo(di);
> -            DeviceState *ds = blk_get_attached_dev(blk);
> +        idebus = &pci_ide->bus[i / 2];
> +        blk = idebus->ifs[i % 2].blk;
>   
> -            blk_drain(blk);
> -            blk_flush(blk);
> -
> -            if (ds) {
> -                blk_detach_dev(blk, ds);
> -            }
> -            pci_ide->bus[di->bus].ifs[di->unit].blk = NULL;
> +        if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) {
>               if (!(i % 2)) {
> -                idedev = pci_ide->bus[di->bus].master;
> +                idedev = idebus->master;
>               } else {
> -                idedev = pci_ide->bus[di->bus].slave;
> +                idedev = idebus->slave;
>               }
> +
> +            blk_drain(blk);
> +            blk_flush(blk);
> +
> +            blk_detach_dev(blk, DEVICE(idedev));
> +            idebus->ifs[i % 2].blk = NULL;
>               idedev->conf.blk = NULL;
>               monitor_remove_blk(blk);
>               blk_unref(blk);
> 

Re: [PATCH v2] xen: rework pci_piix3_xen_ide_unplug

[Anthony PERARD via]

On Tue, Oct 27, 2020 at 01:33:32PM -0400, John Snow wrote:
> On 10/27/20 11:40 AM, Anthony PERARD wrote:
> > From: Anthony PERARD <anthony.perard@citrix.com>
> > 
> > This is to allow IDE disks to be unplugged when adding to QEMU via:
> >      -drive file=/root/disk_file,if=none,id=ide-disk0,format=raw
> >      -device ide-hd,drive=ide-disk0,bus=ide.0,unit=0
> > 
> > as the current code only works for disk added with:
> >      -drive file=/root/disk_file,if=ide,index=0,media=disk,format=raw
> > 
> > Since the code already have the IDE controller as `dev`, we don't need
> > to use the legacy DriveInfo to find all the drive we want to unplug.
> > We can simply use `blk` from the controller, as it kind of was already
> > assume to be the same, by setting it to NULL.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Acked-by: John Snow <jsnow@redhat.com>
> 
> Do you need me to send a PR for this?

No, that's fine, I can do the PR since it's all xen code.

Thanks,

-- 
Anthony PERARD