[Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gerd Hoffmann]

  Hi,

This patch series adds a qdev flag which allows devices being tagged as
not hotpluggable.  It also sets this flag for a number of devices.

Gerd Hoffmann (3):
  qdev: allow devices being tagged as not hotpluggable.
  piix: tag as non-hotpluggable.
  vga: tag as not hotplugable.

 hw/acpi_piix4.c |    2 ++
 hw/cirrus_vga.c |    1 +
 hw/ide/piix.c   |    2 ++
 hw/piix4.c      |    1 +
 hw/piix_pci.c   |    2 ++
 hw/qdev.c       |   16 +++++++++++++---
 hw/qdev.h       |    1 +
 hw/vga-pci.c    |    1 +
 hw/vmware_vga.c |    1 +
 qerror.c        |    4 ++++
 qerror.h        |    3 +++
 11 files changed, 31 insertions(+), 3 deletions(-)

[Qemu-devel] [PATCH 1/3] qdev: allow devices being tagged as not hotpluggable.

[Gerd Hoffmann]

This patch adds a field to DeviceInfo to tag devices as being not
hotpluggable.  Any attempt to plug-in or -out such a device will
throw an error.

This check is done in addition to the check whenever the bus supports
hotplug, i.e. there is no need to tag devices which sit on busses
without hotplug support (ISA for example).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   16 +++++++++++++---
 hw/qdev.h |    1 +
 qerror.c  |    4 ++++
 qerror.h  |    3 +++
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 35858cb..5ef5346 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -234,9 +234,15 @@ DeviceState *qdev_device_add(QemuOpts *opts)
             return NULL;
         }
     }
-    if (qdev_hotplug && !bus->allow_hotplug) {
-        qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
-        return NULL;
+    if (qdev_hotplug) {
+        if (!bus->allow_hotplug) {
+            qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
+            return NULL;
+        }
+        if (info->no_hotplug) {
+            qerror_report(QERR_DEVICE_NO_HOTPLUG, info->name);
+            return NULL;
+        }
     }
 
     /* create device, set properties */
@@ -303,6 +309,10 @@ int qdev_unplug(DeviceState *dev)
         qerror_report(QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
         return -1;
     }
+    if (dev->info->no_hotplug) {
+        qerror_report(QERR_DEVICE_NO_HOTPLUG, dev->info->name);
+        return -1;
+    }
     assert(dev->info->unplug != NULL);
 
     return dev->info->unplug(dev);
diff --git a/hw/qdev.h b/hw/qdev.h
index 579328a..5b57b2b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -144,6 +144,7 @@ struct DeviceInfo {
     size_t size;
     Property *props;
     int no_user;
+    int no_hotplug;
 
     /* callbacks */
     qdev_resetfn reset;
diff --git a/qerror.c b/qerror.c
index ac2cdaf..9d0cdeb 100644
--- a/qerror.c
+++ b/qerror.c
@@ -101,6 +101,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' has no child bus",
     },
     {
+        .error_fmt = QERR_DEVICE_NO_HOTPLUG,
+        .desc      = "Device '%(device)' does not support hotplugging",
+    },
+    {
         .error_fmt = QERR_DUPLICATE_ID,
         .desc      = "Duplicate ID '%(id)' for %(object)",
     },
diff --git a/qerror.h b/qerror.h
index 943a24b..b0f69da 100644
--- a/qerror.h
+++ b/qerror.h
@@ -90,6 +90,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_NO_BUS \
     "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
 
+#define QERR_DEVICE_NO_HOTPLUG \
+    "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
+
 #define QERR_DUPLICATE_ID \
     "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
 
-- 
1.7.1

[Qemu-devel] [PATCH 2/3] piix: tag as not hotpluggable.

[Gerd Hoffmann]

This patch tags all pci devices which belong to the piix3/4 chipsets as
not hotpluggable (Host bridge, ISA bridge, IDE controller, ACPI bridge).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/acpi_piix4.c |    2 ++
 hw/ide/piix.c   |    2 ++
 hw/piix4.c      |    1 +
 hw/piix_pci.c   |    2 ++
 4 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index f549089..a896f65 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -439,6 +439,8 @@ static PCIDeviceInfo piix4_pm_info = {
     .qdev.desc          = "PM",
     .qdev.size          = sizeof(PIIX4PMState),
     .qdev.vmsd          = &vmstate_acpi,
+    .qdev.no_user       = 1,
+    .qdev.no_hotplug    = 1,
     .init               = piix4_pm_initfn,
     .config_write       = pm_write_config,
     .qdev.props         = (Property[]) {
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 07483e8..f46e007 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -184,11 +184,13 @@ static PCIDeviceInfo piix_ide_info[] = {
         .qdev.name    = "piix3-ide",
         .qdev.size    = sizeof(PCIIDEState),
         .qdev.no_user = 1,
+        .qdev.no_hotplug = 1,
         .init         = pci_piix3_ide_initfn,
     },{
         .qdev.name    = "piix4-ide",
         .qdev.size    = sizeof(PCIIDEState),
         .qdev.no_user = 1,
+        .qdev.no_hotplug = 1,
         .init         = pci_piix4_ide_initfn,
     },{
         /* end of list */
diff --git a/hw/piix4.c b/hw/piix4.c
index 5489386..1678898 100644
--- a/hw/piix4.c
+++ b/hw/piix4.c
@@ -113,6 +113,7 @@ static PCIDeviceInfo piix4_info[] = {
         .qdev.desc    = "ISA bridge",
         .qdev.size    = sizeof(PCIDevice),
         .qdev.no_user = 1,
+        .qdev.no_hotplug = 1,
         .init         = piix4_initfn,
     },{
         /* end of list */
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index b5589b9..304a2bd 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -348,6 +348,7 @@ static PCIDeviceInfo i440fx_info[] = {
         .qdev.size    = sizeof(PCII440FXState),
         .qdev.vmsd    = &vmstate_i440fx,
         .qdev.no_user = 1,
+        .qdev.no_hotplug = 1,
         .init         = i440fx_initfn,
         .config_write = i440fx_write_config,
     },{
@@ -356,6 +357,7 @@ static PCIDeviceInfo i440fx_info[] = {
         .qdev.size    = sizeof(PIIX3State),
         .qdev.vmsd    = &vmstate_piix3,
         .qdev.no_user = 1,
+        .qdev.no_hotplug = 1,
         .init         = piix3_initfn,
     },{
         /* end of list */
-- 
1.7.1

[Qemu-devel] [PATCH 3/3] vga: tag as not hotplugable.

[Gerd Hoffmann]

This patch tags all vga cards as not hotpluggable.  The qemu
standard vga will never ever be hotpluggable.  For cirrus + vmware
it might be possible to get that work some day.  Todays we can't
handle that for a number of reasons though.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/cirrus_vga.c |    1 +
 hw/vga-pci.c    |    1 +
 hw/vmware_vga.c |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index b473759..0577e94 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3227,6 +3227,7 @@ static PCIDeviceInfo cirrus_vga_info = {
     .qdev.desc    = "Cirrus CLGD 54xx VGA",
     .qdev.size    = sizeof(PCICirrusVGAState),
     .qdev.vmsd    = &vmstate_pci_cirrus_vga,
+    .qdev.no_hotplug = 1,
     .init         = pci_cirrus_vga_initfn,
     .romfile      = VGABIOS_CIRRUS_FILENAME,
     .config_write = pci_cirrus_write_config,
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 4931eee..31e82d5 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -114,6 +114,7 @@ static PCIDeviceInfo vga_info = {
     .qdev.name    = "VGA",
     .qdev.size    = sizeof(PCIVGAState),
     .qdev.vmsd    = &vmstate_vga_pci,
+    .qdev.no_hotplug = 1,
     .init         = pci_vga_initfn,
     .config_write = pci_vga_write_config,
     .romfile      = "vgabios-stdvga.bin",
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index e852620..4a0525f 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1322,6 +1322,7 @@ static PCIDeviceInfo vmsvga_info = {
     .qdev.name    = "vmware-svga",
     .qdev.size    = sizeof(struct pci_vmsvga_state_s),
     .qdev.vmsd    = &vmstate_vmware_vga,
+    .qdev.no_hotplug = 1,
     .init         = pci_vmsvga_initfn,
     .romfile      = "vgabios-vmware.bin",
 };
-- 
1.7.1

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gleb Natapov]

On Thu, Nov 18, 2010 at 11:45:15AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> This patch series adds a qdev flag which allows devices being tagged as
> not hotpluggable.  It also sets this flag for a number of devices.
> 
Do we want to be able to mark device as not hot-unpluggable from command
like too? Something like this -device blabla,notunplug=no.

> Gerd Hoffmann (3):
>   qdev: allow devices being tagged as not hotpluggable.
>   piix: tag as non-hotpluggable.
>   vga: tag as not hotplugable.
> 
>  hw/acpi_piix4.c |    2 ++
>  hw/cirrus_vga.c |    1 +
>  hw/ide/piix.c   |    2 ++
>  hw/piix4.c      |    1 +
>  hw/piix_pci.c   |    2 ++
>  hw/qdev.c       |   16 +++++++++++++---
>  hw/qdev.h       |    1 +
>  hw/vga-pci.c    |    1 +
>  hw/vmware_vga.c |    1 +
>  qerror.c        |    4 ++++
>  qerror.h        |    3 +++
>  11 files changed, 31 insertions(+), 3 deletions(-)
> 

--
			Gleb.

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gerd Hoffmann]

On 11/18/10 12:01, Gleb Natapov wrote:
> On Thu, Nov 18, 2010 at 11:45:15AM +0100, Gerd Hoffmann wrote:
>>    Hi,
>>
>> This patch series adds a qdev flag which allows devices being tagged as
>> not hotpluggable.  It also sets this flag for a number of devices.
>>
> Do we want to be able to mark device as not hot-unpluggable from command
> like too? Something like this -device blabla,notunplug=no.

Hmm, dunno.  Do you have a example where this would be needed or useful?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gleb Natapov]

On Thu, Nov 18, 2010 at 12:13:21PM +0100, Gerd Hoffmann wrote:
> On 11/18/10 12:01, Gleb Natapov wrote:
> >On Thu, Nov 18, 2010 at 11:45:15AM +0100, Gerd Hoffmann wrote:
> >>   Hi,
> >>
> >>This patch series adds a qdev flag which allows devices being tagged as
> >>not hotpluggable.  It also sets this flag for a number of devices.
> >>
> >Do we want to be able to mark device as not hot-unpluggable from command
> >like too? Something like this -device blabla,notunplug=no.
> 
> Hmm, dunno.  Do you have a example where this would be needed or useful?
> 
Dunno me too. Windows allows to eject any hot-unpluggable device to any
user and in the past we got requirement to disable this and had to build
two BIOSes one with cpu hot-plug support another without. So
hot-pluggability of device looks like management decision (along with
technical one if device can't be actually unplugged).

--
			Gleb.

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gerd Hoffmann]

On 11/18/10 12:20, Gleb Natapov wrote:
> On Thu, Nov 18, 2010 at 12:13:21PM +0100, Gerd Hoffmann wrote:
>> On 11/18/10 12:01, Gleb Natapov wrote:
>>> On Thu, Nov 18, 2010 at 11:45:15AM +0100, Gerd Hoffmann wrote:
>>>>    Hi,
>>>>
>>>> This patch series adds a qdev flag which allows devices being tagged as
>>>> not hotpluggable.  It also sets this flag for a number of devices.
>>>>
>>> Do we want to be able to mark device as not hot-unpluggable from command
>>> like too? Something like this -device blabla,notunplug=no.
>>
>> Hmm, dunno.  Do you have a example where this would be needed or useful?
>>
> Dunno me too. Windows allows to eject any hot-unpluggable device to any
> user and in the past we got requirement to disable this and had to build
> two BIOSes one with cpu hot-plug support another without. So
> hot-pluggability of device looks like management decision (along with
> technical one if device can't be actually unplugged).

For *that* use case well have to do a bit more like dynamically building 
the acpi table which indicates which slots are hot-pluggable and which 
are not.  Which indeed would be useful and would fix the windows xp 
offering me to unplug the piix chipset in the "savely remove hardware" 
menu ;)

But I suspect it also isn't exactly trivial and way behind the scope of 
this little patch set ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gleb Natapov]

On Thu, Nov 18, 2010 at 12:29:07PM +0100, Gerd Hoffmann wrote:
> On 11/18/10 12:20, Gleb Natapov wrote:
> >On Thu, Nov 18, 2010 at 12:13:21PM +0100, Gerd Hoffmann wrote:
> >>On 11/18/10 12:01, Gleb Natapov wrote:
> >>>On Thu, Nov 18, 2010 at 11:45:15AM +0100, Gerd Hoffmann wrote:
> >>>>   Hi,
> >>>>
> >>>>This patch series adds a qdev flag which allows devices being tagged as
> >>>>not hotpluggable.  It also sets this flag for a number of devices.
> >>>>
> >>>Do we want to be able to mark device as not hot-unpluggable from command
> >>>like too? Something like this -device blabla,notunplug=no.
> >>
> >>Hmm, dunno.  Do you have a example where this would be needed or useful?
> >>
> >Dunno me too. Windows allows to eject any hot-unpluggable device to any
> >user and in the past we got requirement to disable this and had to build
> >two BIOSes one with cpu hot-plug support another without. So
> >hot-pluggability of device looks like management decision (along with
> >technical one if device can't be actually unplugged).
> 
> For *that* use case well have to do a bit more like dynamically
> building the acpi table which indicates which slots are
> hot-pluggable and which are not.  Which indeed would be useful and
> would fix the windows xp offering me to unplug the piix chipset in
> the "savely remove hardware" menu ;)
> 
Yes, but management has to specify to us somehow that certain device
is not hotpluggable and notunplug=no looks like good way to do it.

> But I suspect it also isn't exactly trivial and way behind the scope
> of this little patch set ...
> 
If it is not trivial I will not insist.

--
			Gleb.

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Michael Tokarev]

On 18.11.2010 14:42, Gleb Natapov wrote:
[]
>> For *that* use case well have to do a bit more like dynamically
>> building the acpi table which indicates which slots are
>> hot-pluggable and which are not.  Which indeed would be useful and
>> would fix the windows xp offering me to unplug the piix chipset in
>> the "savely remove hardware" menu ;)
>>
> Yes, but management has to specify to us somehow that certain device
> is not hotpluggable and notunplug=no looks like good way to do it.

Can we _please_ get rid of this double negative and specify
it as usual hotplug={on|off} ? :)  It does not matter if in
the code it will be called nohotplug, the parameter parser
function can perform the necessary negation.  nohotplug=no
is just plain wrong ;)

/mjt

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gleb Natapov]

On Thu, Nov 18, 2010 at 03:07:48PM +0300, Michael Tokarev wrote:
> On 18.11.2010 14:42, Gleb Natapov wrote:
> []
> >> For *that* use case well have to do a bit more like dynamically
> >> building the acpi table which indicates which slots are
> >> hot-pluggable and which are not.  Which indeed would be useful and
> >> would fix the windows xp offering me to unplug the piix chipset in
> >> the "savely remove hardware" menu ;)
> >>
> > Yes, but management has to specify to us somehow that certain device
> > is not hotpluggable and notunplug=no looks like good way to do it.
> 
> Can we _please_ get rid of this double negative and specify
> it as usual hotplug={on|off} ? :)  It does not matter if in
> the code it will be called nohotplug, the parameter parser
> function can perform the necessary negation.  nohotplug=no
> is just plain wrong ;)
> 
Don't care about name.

--
			Gleb.

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Anthony Liguori]

On 11/18/2010 04:45 AM, Gerd Hoffmann wrote:
>    Hi,
>
> This patch series adds a qdev flag which allows devices being tagged as
> not hotpluggable.  It also sets this flag for a number of devices.
>
>    

I understand why you're adding this but this is one of those horrible 
abuses of qdev that we really need to avoid.

There are two valid reasons why hotplug is not possible:

1) Hotplugging is not supported by the *slot*.  This is something that 
needs to be exposes through ACPI.  This is not a qdev property, but a 
property of a PCI slot.  It's very important that we do this correctly 
because Windows puts a little icon in the systray that advertises 
quick-removal of devices in slots that support hotplug.

2) The PCI device is soldered to the MB or is otherwise not part of a 
PCI slot.  Again, this is part of the ACPI definition.

Since the PIIX3 lives in slot 1, our ACPI tables should not advertise 
slot 0 or slot 1 as supporting hotplug.

Hotplug information has no business being part of the core qdev 
structures.  Hotplug is a PCI concept and the information needs to live 
at the PCI layer to be meaningfully.

An ideal interface would explicitly allow a user to mark a series of PCI 
slots as no supporting hotplug.  It would be convenient in order to 
ensure that your virtio-net wasn't accidentally ejected by a click-happy 
Windows user.

Regards,

Anthony Liguori

> Gerd Hoffmann (3):
>    qdev: allow devices being tagged as not hotpluggable.
>    piix: tag as non-hotpluggable.
>    vga: tag as not hotplugable.
>
>   hw/acpi_piix4.c |    2 ++
>   hw/cirrus_vga.c |    1 +
>   hw/ide/piix.c   |    2 ++
>   hw/piix4.c      |    1 +
>   hw/piix_pci.c   |    2 ++
>   hw/qdev.c       |   16 +++++++++++++---
>   hw/qdev.h       |    1 +
>   hw/vga-pci.c    |    1 +
>   hw/vmware_vga.c |    1 +
>   qerror.c        |    4 ++++
>   qerror.h        |    3 +++
>   11 files changed, 31 insertions(+), 3 deletions(-)
>
>
>
>    

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gleb Natapov]

On Fri, Nov 19, 2010 at 08:30:35PM -0600, Anthony Liguori wrote:
> On 11/18/2010 04:45 AM, Gerd Hoffmann wrote:
> >   Hi,
> >
> >This patch series adds a qdev flag which allows devices being tagged as
> >not hotpluggable.  It also sets this flag for a number of devices.
> >
> 
> I understand why you're adding this but this is one of those
> horrible abuses of qdev that we really need to avoid.
> 
> There are two valid reasons why hotplug is not possible:
> 
> 1) Hotplugging is not supported by the *slot*.  This is something
> that needs to be exposes through ACPI.  This is not a qdev property,
> but a property of a PCI slot.  It's very important that we do this
> correctly because Windows puts a little icon in the systray that
> advertises quick-removal of devices in slots that support hotplug.
> 
> 2) The PCI device is soldered to the MB or is otherwise not part of
> a PCI slot.  Again, this is part of the ACPI definition.
> 
This patch series introduce "soldered" property for qdev device. It
means that device is not removable from the MB and thus can't be deleted
by issuing delete command in qemu monitor.

> Since the PIIX3 lives in slot 1, our ACPI tables should not
> advertise slot 0 or slot 1 as supporting hotplug.
> 
ACPI table may not advertise it, but it will not prevent removing of
PIIX3 by device_del command (or how it is called this days).

> Hotplug information has no business being part of the core qdev
> structures.  Hotplug is a PCI concept and the information needs to
> live at the PCI layer to be meaningfully.
> 
I am not sure that PCI is the only bus that supports hot-plug. USB and
SCSI support it to and I am sure many others.

> An ideal interface would explicitly allow a user to mark a series of
> PCI slots as no supporting hotplug.  It would be convenient in order
> to ensure that your virtio-net wasn't accidentally ejected by a
> click-happy Windows user.
> 
> Regards,
> 
> Anthony Liguori
> 
> >Gerd Hoffmann (3):
> >   qdev: allow devices being tagged as not hotpluggable.
> >   piix: tag as non-hotpluggable.
> >   vga: tag as not hotplugable.
> >
> >  hw/acpi_piix4.c |    2 ++
> >  hw/cirrus_vga.c |    1 +
> >  hw/ide/piix.c   |    2 ++
> >  hw/piix4.c      |    1 +
> >  hw/piix_pci.c   |    2 ++
> >  hw/qdev.c       |   16 +++++++++++++---
> >  hw/qdev.h       |    1 +
> >  hw/vga-pci.c    |    1 +
> >  hw/vmware_vga.c |    1 +
> >  qerror.c        |    4 ++++
> >  qerror.h        |    3 +++
> >  11 files changed, 31 insertions(+), 3 deletions(-)
> >
> >
> >
> 

--
			Gleb.

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gerd Hoffmann]

> I understand why you're adding this but this is one of those horrible
> abuses of qdev that we really need to avoid.
>
> There are two valid reasons why hotplug is not possible:
>
> 1) Hotplugging is not supported by the *slot*.  This is something that
> needs to be exposes through ACPI. This is not a qdev property, but a
> property of a PCI slot.

Well, yea, right.  Sort of.  The ACPI thing applies to some of the slots 
only.  But, yes, strictly speaking this is a slot not a device property 
in the case of PCI.  Problem is qemu doesn't really has an idea what a 
pci slot is ...

> It's very important that we do this correctly
> because Windows puts a little icon in the systray that advertises
> quick-removal of devices in slots that support hotplug.

Indeed.

> 2) The PCI device is soldered to the MB or is otherwise not part of a
> PCI slot. Again, this is part of the ACPI definition.

(3) The qemu emulation can't handle hot-unplug.

> Since the PIIX3 lives in slot 1, our ACPI tables should not advertise
> slot 0 or slot 1 as supporting hotplug.

They do currently.  Should be easily fixable.

> Hotplug information has no business being part of the core qdev
> structures. Hotplug is a PCI concept and the information needs to live
> at the PCI layer to be meaningfully.

Wrong.  PCI certainly isn't the only bus which supports hotplug.  It 
*does* make sense to handle generic hotplug stuff at qdev level.

> An ideal interface would explicitly allow a user to mark a series of PCI
> slots as no supporting hotplug. It would be convenient in order to
> ensure that your virtio-net wasn't accidentally ejected by a click-happy
> Windows user.

Indeed.  That one is a bit harder I suspect.  Can this be done without 
generating acpi tables dynamically?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gleb Natapov]

On Mon, Nov 22, 2010 at 11:17:10AM +0100, Gerd Hoffmann wrote:
> >An ideal interface would explicitly allow a user to mark a series of PCI
> >slots as no supporting hotplug. It would be convenient in order to
> >ensure that your virtio-net wasn't accidentally ejected by a click-happy
> >Windows user.
> 
> Indeed.  That one is a bit harder I suspect.  Can this be done
> without generating acpi tables dynamically?
> 
I tried and failed :( You can try to :)

--
			Gleb.

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>> I understand why you're adding this but this is one of those horrible
>> abuses of qdev that we really need to avoid.
>>
>> There are two valid reasons why hotplug is not possible:
>>
>> 1) Hotplugging is not supported by the *slot*.  This is something that
>> needs to be exposes through ACPI. This is not a qdev property, but a
>> property of a PCI slot.
>
> Well, yea, right.  Sort of.  The ACPI thing applies to some of the
> slots only.  But, yes, strictly speaking this is a slot not a device
> property in the case of PCI.  Problem is qemu doesn't really has an
> idea what a pci slot is ...

Needs fixing.  Can't see how we can get sane hot plug without a proper
representation of PCI slots in QEMU.

>> It's very important that we do this correctly
>> because Windows puts a little icon in the systray that advertises
>> quick-removal of devices in slots that support hotplug.
>
> Indeed.
>
>> 2) The PCI device is soldered to the MB or is otherwise not part of a
>> PCI slot. Again, this is part of the ACPI definition.
>
> (3) The qemu emulation can't handle hot-unplug.
>
>> Since the PIIX3 lives in slot 1, our ACPI tables should not advertise
>> slot 0 or slot 1 as supporting hotplug.
>
> They do currently.  Should be easily fixable.
>
>> Hotplug information has no business being part of the core qdev
>> structures. Hotplug is a PCI concept and the information needs to live
>> at the PCI layer to be meaningfully.
>
> Wrong.  PCI certainly isn't the only bus which supports hotplug.  It
> *does* make sense to handle generic hotplug stuff at qdev level.

Could the proper place be qbus instead of qdev?

>> An ideal interface would explicitly allow a user to mark a series of PCI
>> slots as no supporting hotplug. It would be convenient in order to
>> ensure that your virtio-net wasn't accidentally ejected by a click-happy
>> Windows user.
>
> Indeed.  That one is a bit harder I suspect.  Can this be done without
> generating acpi tables dynamically?

Re: [Qemu-devel] [PATCH 0/3] add hotplug opt-out option for devices.

[Gerd Hoffmann]

   Hi,

>> Wrong.  PCI certainly isn't the only bus which supports hotplug.  It
>> *does* make sense to handle generic hotplug stuff at qdev level.
>
> Could the proper place be qbus instead of qdev?

No.  But PCI is the only bus where some devices are hot-pluggable and 
some are not.  On all other busses it is either all or no devices.  So 
moving to pci is an option (patches for that are on the list).

cheers,
   Gerd

[Qemu-devel] [PATCH 0/3] add hotplug opt-out option for pci devices.

[Gerd Hoffmann]

  Hi,

This patch series adds a flag which allows pci devices being tagged
as not hotpluggable.  It also sets this flag for a number of devices.

cheers,
  Gerd

Gerd Hoffmann (3):
  pci: allow devices being tagged as not hotpluggable.
  piix: tag as not hotpluggable.
  vga: tag as not hotplugable.

 hw/acpi_piix4.c |    2 ++
 hw/cirrus_vga.c |    1 +
 hw/ide/piix.c   |    2 ++
 hw/pci.c        |   10 ++++++++++
 hw/pci.h        |    3 +++
 hw/piix4.c      |    1 +
 hw/piix_pci.c   |    2 ++
 hw/vga-pci.c    |    1 +
 hw/vmware_vga.c |    1 +
 qerror.c        |    4 ++++
 qerror.h        |    3 +++
 11 files changed, 30 insertions(+), 0 deletions(-)

[Qemu-devel] [PATCH 3/3] vga: tag as not hotplugable.

[Gerd Hoffmann]

This patch tags all vga cards as not hotpluggable.  The qemu
standard vga will never ever be hotpluggable.  For cirrus + vmware
it might be possible to get that work some day.  Todays we can't
handle that for a number of reasons though.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/cirrus_vga.c |    1 +
 hw/vga-pci.c    |    1 +
 hw/vmware_vga.c |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index aadc56f..1936730 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3223,6 +3223,7 @@ static PCIDeviceInfo cirrus_vga_info = {
     .qdev.desc    = "Cirrus CLGD 54xx VGA",
     .qdev.size    = sizeof(PCICirrusVGAState),
     .qdev.vmsd    = &vmstate_pci_cirrus_vga,
+    .no_hotplug   = 1,
     .init         = pci_cirrus_vga_initfn,
     .romfile      = VGABIOS_CIRRUS_FILENAME,
     .config_write = pci_cirrus_write_config,
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 791ca22..ce9ec45 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -110,6 +110,7 @@ static PCIDeviceInfo vga_info = {
     .qdev.name    = "VGA",
     .qdev.size    = sizeof(PCIVGAState),
     .qdev.vmsd    = &vmstate_vga_pci,
+    .no_hotplug   = 1,
     .init         = pci_vga_initfn,
     .config_write = pci_vga_write_config,
     .romfile      = "vgabios-stdvga.bin",
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index d0f4e1b..bcd02d2 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1318,6 +1318,7 @@ static PCIDeviceInfo vmsvga_info = {
     .qdev.name    = "vmware-svga",
     .qdev.size    = sizeof(struct pci_vmsvga_state_s),
     .qdev.vmsd    = &vmstate_vmware_vga,
+    .no_hotplug   = 1,
     .init         = pci_vmsvga_initfn,
     .romfile      = "vgabios-vmware.bin",
 };
-- 
1.7.1