| Message ID | AANLkTil60phutH1zA9qn9fsI83frmcA177LqFR2zmJUL@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Marcos Oviedo <moviedo.maillist@gmail.com> writes: > This adds a way to force the removal/unplug of previously added pci > devices when ACPI-based hotplug mechanism is not present. > > Signed-off-by: Marcos Oviedo <moviedo@gmail.com> If this makes sense for pci_del (I'm not passing judgement), then we need it for device_del as well.
On 06/09/10 07:37, Marcos Oviedo wrote: > This adds a way to force the removal/unplug of previously added pci > devices when ACPI-based hotplug mechanism is not present. Point being? If your guest can't handle pci hotplug it is pretty useless to plug in hardware in the first place. If your guest supports pci hotplug it will be quite upset if you zap the hardware without asking via ACPI. cheers, Gerd
On Wed, Jun 9, 2010 at 4:38 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: > On 06/09/10 07:37, Marcos Oviedo wrote: > >> This adds a way to force the removal/unplug of previously added pci >> devices when ACPI-based hotplug mechanism is not present. >> > > Point being? > > If your guest can't handle pci hotplug it is pretty useless to plug in > hardware in the first place. > > If your guest supports pci hotplug it will be quite upset if you zap the > hardware without asking via ACPI. > This make sense when you mistakenly add a pci device on a -s -S scenario, like the scenario described on the following bug: https://bugs.launchpad.net/qemu/+bug/544367. When ACPI-based hotplug support is present on the guest and we run pci_del with the force option, the hotplug events will still be generated to the guest and the guest still will trigger the EJx event, which will end by calling pciej_write() on qemu side. This function will do nothing on a -f and pci hotplug support scenario, as the pci device was previously removed by pci_del. Thanks! Marcos > > cheers, > Gerd > >
Hi, > This make sense when you mistakenly add a pci device on a -s -S > scenario, like the scenario described on the following bug: > https://bugs.launchpad.net/qemu/+bug/544367. It doesn't IMHO. > When ACPI-based hotplug support is present on the guest and we run > pci_del with the force option, the hotplug events will still be > generated to the guest and the guest still will trigger the EJx event, > which will end by calling pciej_write() on qemu side. This function will > do nothing on a -f and pci hotplug support scenario, as the pci device > was previously removed by pci_del. And in case the guest wants to do anything (like flushing dirty buffers) before triggering the EJx event it will horribly fail. If the guest is stopped while unplugging the device the unplug should happen as soon as the guest is unpaused. cheers, Gerd
On 06/09/2010 09:27 AM, Gerd Hoffmann wrote: > Hi, > >> This make sense when you mistakenly add a pci device on a -s -S >> scenario, like the scenario described on the following bug: >> https://bugs.launchpad.net/qemu/+bug/544367. > > It doesn't IMHO. > >> When ACPI-based hotplug support is present on the guest and we run >> pci_del with the force option, the hotplug events will still be >> generated to the guest and the guest still will trigger the EJx event, >> which will end by calling pciej_write() on qemu side. This function will >> do nothing on a -f and pci hotplug support scenario, as the pci device >> was previously removed by pci_del. > > And in case the guest wants to do anything (like flushing dirty > buffers) before triggering the EJx event it will horribly fail. > > If the guest is stopped while unplugging the device the unplug should > happen as soon as the guest is unpaused. This is a case where the fundamental problem is that the pci_del command should block until the guest has actually responded to the request. pci_del returning with no error and yet not having the operation complete is certainly a usability issue. Regards, Anthony Liguori > cheers, > Gerd > > >
Hi, >> If the guest is stopped while unplugging the device the unplug should >> happen as soon as the guest is unpaused. > > This is a case where the fundamental problem is that the pci_del command > should block until the guest has actually responded to the request. You can't block. Unplug might never ever happen for various reasons. IMHO the only sane way to handle it is sending a event when the unplug is done. > pci_del returning with no error and yet not having the operation > complete is certainly a usability issue. There simply is no clear error condition. If the guest didn't respond (yet) you don't know whenever it just needs a bit more time to shutdown the device or if unplugging the device failed. cheers, Gerd
Anthony Liguori <anthony@codemonkey.ws> writes: > On 06/09/2010 09:27 AM, Gerd Hoffmann wrote: >> Hi, >> >>> This make sense when you mistakenly add a pci device on a -s -S >>> scenario, like the scenario described on the following bug: >>> https://bugs.launchpad.net/qemu/+bug/544367. >> >> It doesn't IMHO. >> >>> When ACPI-based hotplug support is present on the guest and we run >>> pci_del with the force option, the hotplug events will still be >>> generated to the guest and the guest still will trigger the EJx event, >>> which will end by calling pciej_write() on qemu side. This function will >>> do nothing on a -f and pci hotplug support scenario, as the pci device >>> was previously removed by pci_del. >> >> And in case the guest wants to do anything (like flushing dirty >> buffers) before triggering the EJx event it will horribly fail. >> >> If the guest is stopped while unplugging the device the unplug >> should happen as soon as the guest is unpaused. > > This is a case where the fundamental problem is that the pci_del > command should block until the guest has actually responded to the > request. > > pci_del returning with no error and yet not having the operation > complete is certainly a usability issue. s/pci_del/device_del/g :) What should device_del do? Wait until ACPI reports that the guest has processed the unplug event? What if the guest doesn't? Hanging indefinitely is not an option. Can we reliably detect this case?
On Tue, 15 Jun 2010 11:03:13 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Anthony Liguori <anthony@codemonkey.ws> writes: > > > On 06/09/2010 09:27 AM, Gerd Hoffmann wrote: > >> Hi, > >> > >>> This make sense when you mistakenly add a pci device on a -s -S > >>> scenario, like the scenario described on the following bug: > >>> https://bugs.launchpad.net/qemu/+bug/544367. > >> > >> It doesn't IMHO. > >> > >>> When ACPI-based hotplug support is present on the guest and we run > >>> pci_del with the force option, the hotplug events will still be > >>> generated to the guest and the guest still will trigger the EJx event, > >>> which will end by calling pciej_write() on qemu side. This function will > >>> do nothing on a -f and pci hotplug support scenario, as the pci device > >>> was previously removed by pci_del. > >> > >> And in case the guest wants to do anything (like flushing dirty > >> buffers) before triggering the EJx event it will horribly fail. > >> > >> If the guest is stopped while unplugging the device the unplug > >> should happen as soon as the guest is unpaused. > > > > This is a case where the fundamental problem is that the pci_del > > command should block until the guest has actually responded to the > > request. > > > > pci_del returning with no error and yet not having the operation > > complete is certainly a usability issue. > > s/pci_del/device_del/g :) > > What should device_del do? Wait until ACPI reports that the guest has > processed the unplug event? What if the guest doesn't? Hanging > indefinitely is not an option. Can we reliably detect this case? This is a general question for all commands that can take way too long or never return. For QMP the question is whether we should handle this in QEMU or in the client. Ie, if the guest doesn't respond the client could detect that and cancel the async command. For HMP we could just live with that or suspend the shell and allow the user to cancel the operation (eg. ctrl+c) and the obvious alternative is to have timeouts, allowing the user to set them.
On 06/17/2010 01:15 PM, Luiz Capitulino wrote: > This is a general question for all commands that can take way too long > or never return. > > For QMP the question is whether we should handle this in QEMU or in the > client. Ie, if the guest doesn't respond the client could detect that > and cancel the async command. > Exactly. It's no different than a migration that takes too long. > For HMP we could just live with that or suspend the shell and allow the > user to cancel the operation (eg. ctrl+c) and the obvious alternative is to > have timeouts, allowing the user to set them. > Yeah, ctrl+c to cancel would be a very nice feature. Regards, Anthony Liguori
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c index a8f3df1..1007e5b 100644 --- a/hw/pci-hotplug.c +++ b/hw/pci-hotplug.c @@ -260,11 +260,12 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict) } #endif -int pci_device_hot_remove(Monitor *mon, const char *pci_addr) +int pci_device_hot_remove(Monitor *mon, const char *pci_addr, const int forced) { PCIDevice *d; int dom, bus; unsigned slot; + int ret = -1; if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) { return -1; @@ -275,10 +276,19 @@ int pci_device_hot_remove(Monitor *mon, const char *pci_addr) monitor_printf(mon, "slot %d empty\n", slot); return -1; } - return qdev_unplug(&d->qdev); + + if (forced) { + qdev_free(&d->qdev); + ret = 0; + } + else { + ret = qdev_unplug(&d->qdev); + } + + return ret; } void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict) { - pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr")); + pci_device_hot_remove(mon, qdict_get_str(qdict, "pci_addr"), qdict_get_int(qdict, "force")); } diff --git a/qemu-monitor.hx b/qemu-monitor.hx index f6a94f2..ce8cddd 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -1140,8 +1140,8 @@ ETEXI #if defined(TARGET_I386) { .name = "pci_del", - .args_type = "pci_addr:s", - .params = "[[<domain>:]<bus>:]<slot>", + .args_type = "pci_addr:s,force:-f", + .params = "[[<domain>:]<bus>:]<slot> [-f]", .help = "hot remove PCI device", .mhandler.cmd = do_pci_device_hot_remove, }, diff --git a/sysemu.h b/sysemu.h index 879446a..22303b3 100644 --- a/sysemu.h +++ b/sysemu.h @@ -202,7 +202,7 @@ DriveInfo *add_init_drive(const char *opts); /* pci-hotplug */ void pci_device_hot_add(Monitor *mon, const QDict *qdict); void drive_hot_add(Monitor *mon, const QDict *qdict); -int pci_device_hot_remove(Monitor *mon, const char *pci_addr); +int pci_device_hot_remove(Monitor *mon, const char *pci_addr, const int forced); void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
This adds a way to force the removal/unplug of previously added pci devices when ACPI-based hotplug mechanism is not present. Signed-off-by: Marcos Oviedo <moviedo@gmail.com> --- hw/pci-hotplug.c | 16 +++++++++++++--- qemu-monitor.hx | 4 ++-- sysemu.h | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) /* serial ports */