| Message ID | 20200918145256.1886250-1-anthony.perard@citrix.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: rework pci_piix3_xen_ide_unplug | expand |
Patchew URL: https://patchew.org/QEMU/20200918145256.1886250-1-anthony.perard@citrix.com/ Hi, This series seems to have some coding style problems. See output below for more information: N/A. Internal error while reading log file The full log is available at http://patchew.org/logs/20200918145256.1886250-1-anthony.perard@citrix.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
> -----Original Message----- > From: Anthony PERARD <anthony.perard@citrix.com> > Sent: 18 September 2020 15:53 > To: qemu-devel@nongnu.org > Cc: Paul Durrant <paul@xen.org>; Stefano Stabellini <sstabellini@kernel.org>; Anthony PERARD > <anthony.perard@citrix.com>; John Snow <jsnow@redhat.com>; qemu-block@nongnu.org > Subject: [PATCH] xen: rework pci_piix3_xen_ide_unplug > > 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> > --- > 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..16fcbe58d6fe 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) { Spaces around '%' > if (!(i % 2)) { > - idedev = pci_ide->bus[di->bus].master; > + idedev = idebus->master; > } else { > - idedev = pci_ide->bus[di->bus].slave; > + idedev = idebus->slave; > } Perhaps use a ternary operator here rather than if/else. Also there are 3 uses 'i % 2' so I wonder whether a local 'device index' variable might be neater. Paul > + > + 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
diff --git a/hw/ide/piix.c b/hw/ide/piix.c index b402a936362b..16fcbe58d6fe 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);
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> --- hw/ide/piix.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)