qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device
@ 2018-12-10 19:14 Tony Krowiak
  2018-12-11 15:23 ` Igor Mammedov
  0 siblings, 1 reply; 6+ messages in thread
From: Tony Krowiak @ 2018-12-10 19:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, pasic, armbru, f4bug, pbonzini, eblake, thuth,
	Tony Krowiak

If the maximum number of devices allowed on a bus is 1 and a device
which is plugged into the bus is subsequently unplugged, attempting to replug
the device fails with error "Bus 'xxx' does not support hotplugging".
The "error" is detected in the qbus_is_full(BusState *bus) function
(qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
root of the problem is that the bus->max_index is not decremented when a device
is unplugged from the bus. This patch fixes that problem.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
---
 hw/core/qdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27c2..b35b0bf27925 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
             snprintf(name, sizeof(name), "child[%d]", kid->index);
             QTAILQ_REMOVE(&bus->children, kid, sibling);
 
+            bus->max_index--;
+
             /* This gives back ownership of kid->child back to us.  */
             object_property_del(OBJECT(bus), name, NULL);
             object_unref(OBJECT(kid->child));
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device
  2018-12-10 19:14 [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device Tony Krowiak
@ 2018-12-11 15:23 ` Igor Mammedov
  2018-12-11 19:41   ` Tony Krowiak
  0 siblings, 1 reply; 6+ messages in thread
From: Igor Mammedov @ 2018-12-11 15:23 UTC (permalink / raw)
  To: Tony Krowiak
  Cc: qemu-devel, thuth, armbru, f4bug, pasic, borntraeger, pbonzini

On Mon, 10 Dec 2018 14:14:14 -0500
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> If the maximum number of devices allowed on a bus is 1 and a device
> which is plugged into the bus is subsequently unplugged, attempting to replug
> the device fails with error "Bus 'xxx' does not support hotplugging".
> The "error" is detected in the qbus_is_full(BusState *bus) function
> (qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
> root of the problem is that the bus->max_index is not decremented when a device
> is unplugged from the bus. This patch fixes that problem.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> ---
>  hw/core/qdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6b3cc55b27c2..b35b0bf27925 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
>              snprintf(name, sizeof(name), "child[%d]", kid->index);
>              QTAILQ_REMOVE(&bus->children, kid, sibling);
>  
> +            bus->max_index--;
that might cause problem when bus is allowed to has more than 1 device
and unplugged device is not the last one.
In that case bus_add_child() might create a child with duplicate name.

> +
>              /* This gives back ownership of kid->child back to us.  */
>              object_property_del(OBJECT(bus), name, NULL);
>              object_unref(OBJECT(kid->child));

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device
  2018-12-11 15:23 ` Igor Mammedov
@ 2018-12-11 19:41   ` Tony Krowiak
  2018-12-13 12:09     ` Igor Mammedov
  0 siblings, 1 reply; 6+ messages in thread
From: Tony Krowiak @ 2018-12-11 19:41 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: qemu-devel, thuth, armbru, f4bug, pasic, borntraeger, pbonzini

On 12/11/18 10:23 AM, Igor Mammedov wrote:
> On Mon, 10 Dec 2018 14:14:14 -0500
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> 
>> If the maximum number of devices allowed on a bus is 1 and a device
>> which is plugged into the bus is subsequently unplugged, attempting to replug
>> the device fails with error "Bus 'xxx' does not support hotplugging".
>> The "error" is detected in the qbus_is_full(BusState *bus) function
>> (qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
>> root of the problem is that the bus->max_index is not decremented when a device
>> is unplugged from the bus. This patch fixes that problem.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   hw/core/qdev.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 6b3cc55b27c2..b35b0bf27925 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
>>               snprintf(name, sizeof(name), "child[%d]", kid->index);
>>               QTAILQ_REMOVE(&bus->children, kid, sibling);
>>   
>> +            bus->max_index--;
> that might cause problem when bus is allowed to has more than 1 device
> and unplugged device is not the last one.
> In that case bus_add_child() might create a child with duplicate name.

I see what you are saying. The max_index is assigned to the child device
index in the bus_add_child() function. The child device index is used to
generate a unique name for the child device. The generated name is then
used to link the child as a property to the bus. When the child is
removed from the bus, the name is regenerated from the child device
index and the named property is . It would therefore appear that the
real purpose of the max_index is to generate indexes for children of
the bus thus ensuring each child has a unique index. In other words,
the max_index value is only tangentially connected to indexing the list
of children.

This results in a disconnect between the usage of the max_index value
when adding and removing a child from the bus, and the check in the
qbus_is_full() function which compares the max_index to the maximum
number of devices allowed on the bus. If a child has been removed from
the bus, the max_index value does not indicate whether the bus is
full, it only specifies the index to be assigned to the next child to be
added to the bus.

To resolve this problem, I propose the following:

Add the following field to struct BusState (include/hw/qdev_core.h):

struct BusState {
     Object obj;
     DeviceState *parent;
     char *name;
     HotplugHandler *hotplug_handler;
     int max_index;
     bool realized;
+    int num_children;
     QTAILQ_HEAD(ChildrenHead, BusChild) children;
     QLIST_ENTRY(BusState) sibling;
};

Add the following lines of code to the add/remove child functions in
hw/core/qdev.c:

static void bus_add_child(BusState *bus, DeviceState *child)
{
     char name[32];
     BusChild *kid = g_malloc0(sizeof(*kid));

     kid->index = bus->max_index++;
     kid->child = child;
     object_ref(OBJECT(kid->child));

     QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);

     /* This transfers ownership of kid->child to the property.  */
     snprintf(name, sizeof(name), "child[%d]", kid->index);
     object_property_add_link(OBJECT(bus), name,
                              object_get_typename(OBJECT(child)),
                              (Object **)&kid->child,
                              NULL, /* read-only property */
                              0, /* return ownership on prop deletion */
                              NULL);

+    bus->num_children++;
}

static void bus_remove_child(BusState *bus, DeviceState *child)
{
     BusChild *kid;

     QTAILQ_FOREACH(kid, &bus->children, sibling) {
         if (kid->child == child) {
             char name[32];

             snprintf(name, sizeof(name), "child[%d]", kid->index);
             QTAILQ_REMOVE(&bus->children, kid, sibling);

             /* This gives back ownership of kid->child back to us.  */
             object_property_del(OBJECT(bus), name, NULL);
             object_unref(OBJECT(kid->child));
             g_free(kid);

+           bus->num_children--;

             return;
         }
     }
}

Change the line of code in the qbus_is_full() function in
qdev_monitor.c:


static inline bool qbus_is_full(BusState *bus)
{
     BusClass *bus_class = BUS_GET_CLASS(bus);
-    return bus_class->max_dev &&
-           bus->max_index >= bus_class->max_dev;
+    return bus_class->max_dev &&
+           bus->num_children >= bus_class->max_dev;
}



> 
>> +
>>               /* This gives back ownership of kid->child back to us.  */
>>               object_property_del(OBJECT(bus), name, NULL);
>>               object_unref(OBJECT(kid->child));
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device
  2018-12-11 19:41   ` Tony Krowiak
@ 2018-12-13 12:09     ` Igor Mammedov
  2018-12-13 13:03       ` Halil Pasic
  0 siblings, 1 reply; 6+ messages in thread
From: Igor Mammedov @ 2018-12-13 12:09 UTC (permalink / raw)
  To: Tony Krowiak
  Cc: thuth, armbru, f4bug, qemu-devel, pasic, borntraeger, pbonzini

On Tue, 11 Dec 2018 14:41:00 -0500
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> On 12/11/18 10:23 AM, Igor Mammedov wrote:
> > On Mon, 10 Dec 2018 14:14:14 -0500
> > Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> >   
> >> If the maximum number of devices allowed on a bus is 1 and a device
> >> which is plugged into the bus is subsequently unplugged, attempting to replug
> >> the device fails with error "Bus 'xxx' does not support hotplugging".
> >> The "error" is detected in the qbus_is_full(BusState *bus) function
> >> (qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
> >> root of the problem is that the bus->max_index is not decremented when a device
> >> is unplugged from the bus. This patch fixes that problem.
> >>
> >> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> >> ---
> >>   hw/core/qdev.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >> index 6b3cc55b27c2..b35b0bf27925 100644
> >> --- a/hw/core/qdev.c
> >> +++ b/hw/core/qdev.c
> >> @@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
> >>               snprintf(name, sizeof(name), "child[%d]", kid->index);
> >>               QTAILQ_REMOVE(&bus->children, kid, sibling);
> >>   
> >> +            bus->max_index--;  
> > that might cause problem when bus is allowed to has more than 1 device
> > and unplugged device is not the last one.
> > In that case bus_add_child() might create a child with duplicate name.  
> 
> I see what you are saying. The max_index is assigned to the child device
> index in the bus_add_child() function. The child device index is used to
> generate a unique name for the child device. The generated name is then
> used to link the child as a property to the bus. When the child is
> removed from the bus, the name is regenerated from the child device
> index and the named property is . It would therefore appear that the
> real purpose of the max_index is to generate indexes for children of
> the bus thus ensuring each child has a unique index. In other words,
> the max_index value is only tangentially connected to indexing the list
> of children.
> 
> This results in a disconnect between the usage of the max_index value
> when adding and removing a child from the bus, and the check in the
> qbus_is_full() function which compares the max_index to the maximum
> number of devices allowed on the bus. If a child has been removed from
> the bus, the max_index value does not indicate whether the bus is
> full, it only specifies the index to be assigned to the next child to be
> added to the bus.
> 
> To resolve this problem, I propose the following:
> 
> Add the following field to struct BusState (include/hw/qdev_core.h):
> 
> struct BusState {
>      Object obj;
>      DeviceState *parent;
>      char *name;
>      HotplugHandler *hotplug_handler;
>      int max_index;
>      bool realized;
> +    int num_children;
>      QTAILQ_HEAD(ChildrenHead, BusChild) children;
>      QLIST_ENTRY(BusState) sibling;
> };
> 
> Add the following lines of code to the add/remove child functions in
> hw/core/qdev.c:
> 
> static void bus_add_child(BusState *bus, DeviceState *child)
> {
>      char name[32];
>      BusChild *kid = g_malloc0(sizeof(*kid));
> 
>      kid->index = bus->max_index++;
>      kid->child = child;
>      object_ref(OBJECT(kid->child));
> 
>      QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
> 
>      /* This transfers ownership of kid->child to the property.  */
>      snprintf(name, sizeof(name), "child[%d]", kid->index);
>      object_property_add_link(OBJECT(bus), name,
>                               object_get_typename(OBJECT(child)),
>                               (Object **)&kid->child,
>                               NULL, /* read-only property */
>                               0, /* return ownership on prop deletion */
>                               NULL);
> 
> +    bus->num_children++;
> }
> 
> static void bus_remove_child(BusState *bus, DeviceState *child)
> {
>      BusChild *kid;
> 
>      QTAILQ_FOREACH(kid, &bus->children, sibling) {
>          if (kid->child == child) {
>              char name[32];
> 
>              snprintf(name, sizeof(name), "child[%d]", kid->index);
>              QTAILQ_REMOVE(&bus->children, kid, sibling);
> 
>              /* This gives back ownership of kid->child back to us.  */
>              object_property_del(OBJECT(bus), name, NULL);
>              object_unref(OBJECT(kid->child));
>              g_free(kid);
> 
> +           bus->num_children--;
> 
>              return;
>          }
>      }
> }
> 
> Change the line of code in the qbus_is_full() function in
> qdev_monitor.c:
> 
> 
> static inline bool qbus_is_full(BusState *bus)
> {
>      BusClass *bus_class = BUS_GET_CLASS(bus);
> -    return bus_class->max_dev &&
> -           bus->max_index >= bus_class->max_dev;
> +    return bus_class->max_dev &&
> +           bus->num_children >= bus_class->max_dev;
> }
> 

looks good to me
[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device
  2018-12-13 12:09     ` Igor Mammedov
@ 2018-12-13 13:03       ` Halil Pasic
  2018-12-13 16:10         ` Tony Krowiak
  0 siblings, 1 reply; 6+ messages in thread
From: Halil Pasic @ 2018-12-13 13:03 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Tony Krowiak, thuth, armbru, f4bug, qemu-devel, borntraeger,
	pbonzini

On Thu, 13 Dec 2018 13:09:59 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Tue, 11 Dec 2018 14:41:00 -0500
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> 
> > On 12/11/18 10:23 AM, Igor Mammedov wrote:
> > > On Mon, 10 Dec 2018 14:14:14 -0500
> > > Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> > >   
> > >> If the maximum number of devices allowed on a bus is 1 and a device
> > >> which is plugged into the bus is subsequently unplugged, attempting to replug
> > >> the device fails with error "Bus 'xxx' does not support hotplugging".
> > >> The "error" is detected in the qbus_is_full(BusState *bus) function
> > >> (qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
> > >> root of the problem is that the bus->max_index is not decremented when a device
> > >> is unplugged from the bus. This patch fixes that problem.
> > >>
> > >> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> > >> ---
> > >>   hw/core/qdev.c | 2 ++
> > >>   1 file changed, 2 insertions(+)
> > >>
> > >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > >> index 6b3cc55b27c2..b35b0bf27925 100644
> > >> --- a/hw/core/qdev.c
> > >> +++ b/hw/core/qdev.c
> > >> @@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
> > >>               snprintf(name, sizeof(name), "child[%d]", kid->index);
> > >>               QTAILQ_REMOVE(&bus->children, kid, sibling);
> > >>   
> > >> +            bus->max_index--;  
> > > that might cause problem when bus is allowed to has more than 1 device
> > > and unplugged device is not the last one.
> > > In that case bus_add_child() might create a child with duplicate name.  
> > 
> > I see what you are saying. The max_index is assigned to the child device
> > index in the bus_add_child() function. The child device index is used to
> > generate a unique name for the child device. The generated name is then
> > used to link the child as a property to the bus. When the child is
> > removed from the bus, the name is regenerated from the child device
> > index and the named property is . It would therefore appear that the
> > real purpose of the max_index is to generate indexes for children of
> > the bus thus ensuring each child has a unique index. In other words,
> > the max_index value is only tangentially connected to indexing the list
> > of children.
> > 
> > This results in a disconnect between the usage of the max_index value
> > when adding and removing a child from the bus, and the check in the
> > qbus_is_full() function which compares the max_index to the maximum
> > number of devices allowed on the bus. If a child has been removed from
> > the bus, the max_index value does not indicate whether the bus is
> > full, it only specifies the index to be assigned to the next child to be
> > added to the bus.
> > 
> > To resolve this problem, I propose the following:
> > 
> > Add the following field to struct BusState (include/hw/qdev_core.h):
> > 
> > struct BusState {
> >      Object obj;
> >      DeviceState *parent;
> >      char *name;
> >      HotplugHandler *hotplug_handler;
> >      int max_index;
> >      bool realized;
> > +    int num_children;
> >      QTAILQ_HEAD(ChildrenHead, BusChild) children;
> >      QLIST_ENTRY(BusState) sibling;
> > };
> > 
> > Add the following lines of code to the add/remove child functions in
> > hw/core/qdev.c:
> > 
> > static void bus_add_child(BusState *bus, DeviceState *child)
> > {
> >      char name[32];
> >      BusChild *kid = g_malloc0(sizeof(*kid));
> > 
> >      kid->index = bus->max_index++;
> >      kid->child = child;
> >      object_ref(OBJECT(kid->child));
> > 
> >      QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
> > 
> >      /* This transfers ownership of kid->child to the property.  */
> >      snprintf(name, sizeof(name), "child[%d]", kid->index);
> >      object_property_add_link(OBJECT(bus), name,
> >                               object_get_typename(OBJECT(child)),
> >                               (Object **)&kid->child,
> >                               NULL, /* read-only property */
> >                               0, /* return ownership on prop deletion */
> >                               NULL);
> > 
> > +    bus->num_children++;
> > }
> > 
> > static void bus_remove_child(BusState *bus, DeviceState *child)
> > {
> >      BusChild *kid;
> > 
> >      QTAILQ_FOREACH(kid, &bus->children, sibling) {
> >          if (kid->child == child) {
> >              char name[32];
> > 
> >              snprintf(name, sizeof(name), "child[%d]", kid->index);
> >              QTAILQ_REMOVE(&bus->children, kid, sibling);
> > 
> >              /* This gives back ownership of kid->child back to us.  */
> >              object_property_del(OBJECT(bus), name, NULL);
> >              object_unref(OBJECT(kid->child));
> >              g_free(kid);
> > 
> > +           bus->num_children--;
> > 
> >              return;
> >          }
> >      }
> > }
> > 
> > Change the line of code in the qbus_is_full() function in
> > qdev_monitor.c:
> > 
> > 
> > static inline bool qbus_is_full(BusState *bus)
> > {
> >      BusClass *bus_class = BUS_GET_CLASS(bus);
> > -    return bus_class->max_dev &&
> > -           bus->max_index >= bus_class->max_dev;
> > +    return bus_class->max_dev &&
> > +           bus->num_children >= bus_class->max_dev;
> > }
> > 
> 
> looks good to me
> [...]
> 

I agree, the second proposal looks reasonable. Can you send a proper
patch so I can r-b it?

Halil

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device
  2018-12-13 13:03       ` Halil Pasic
@ 2018-12-13 16:10         ` Tony Krowiak
  0 siblings, 0 replies; 6+ messages in thread
From: Tony Krowiak @ 2018-12-13 16:10 UTC (permalink / raw)
  To: Halil Pasic, Igor Mammedov
  Cc: thuth, armbru, f4bug, qemu-devel, borntraeger, pbonzini

On 12/13/18 8:03 AM, Halil Pasic wrote:
> On Thu, 13 Dec 2018 13:09:59 +0100
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
>> On Tue, 11 Dec 2018 14:41:00 -0500
>> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>>
>>> On 12/11/18 10:23 AM, Igor Mammedov wrote:
>>>> On Mon, 10 Dec 2018 14:14:14 -0500
>>>> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>>>>    
>>>>> If the maximum number of devices allowed on a bus is 1 and a device
>>>>> which is plugged into the bus is subsequently unplugged, attempting to replug
>>>>> the device fails with error "Bus 'xxx' does not support hotplugging".
>>>>> The "error" is detected in the qbus_is_full(BusState *bus) function
>>>>> (qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
>>>>> root of the problem is that the bus->max_index is not decremented when a device
>>>>> is unplugged from the bus. This patch fixes that problem.
>>>>>
>>>>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>>>>> ---
>>>>>    hw/core/qdev.c | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>>> index 6b3cc55b27c2..b35b0bf27925 100644
>>>>> --- a/hw/core/qdev.c
>>>>> +++ b/hw/core/qdev.c
>>>>> @@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
>>>>>                snprintf(name, sizeof(name), "child[%d]", kid->index);
>>>>>                QTAILQ_REMOVE(&bus->children, kid, sibling);
>>>>>    
>>>>> +            bus->max_index--;
>>>> that might cause problem when bus is allowed to has more than 1 device
>>>> and unplugged device is not the last one.
>>>> In that case bus_add_child() might create a child with duplicate name.
>>>
>>> I see what you are saying. The max_index is assigned to the child device
>>> index in the bus_add_child() function. The child device index is used to
>>> generate a unique name for the child device. The generated name is then
>>> used to link the child as a property to the bus. When the child is
>>> removed from the bus, the name is regenerated from the child device
>>> index and the named property is . It would therefore appear that the
>>> real purpose of the max_index is to generate indexes for children of
>>> the bus thus ensuring each child has a unique index. In other words,
>>> the max_index value is only tangentially connected to indexing the list
>>> of children.
>>>
>>> This results in a disconnect between the usage of the max_index value
>>> when adding and removing a child from the bus, and the check in the
>>> qbus_is_full() function which compares the max_index to the maximum
>>> number of devices allowed on the bus. If a child has been removed from
>>> the bus, the max_index value does not indicate whether the bus is
>>> full, it only specifies the index to be assigned to the next child to be
>>> added to the bus.
>>>
>>> To resolve this problem, I propose the following:
>>>
>>> Add the following field to struct BusState (include/hw/qdev_core.h):
>>>
>>> struct BusState {
>>>       Object obj;
>>>       DeviceState *parent;
>>>       char *name;
>>>       HotplugHandler *hotplug_handler;
>>>       int max_index;
>>>       bool realized;
>>> +    int num_children;
>>>       QTAILQ_HEAD(ChildrenHead, BusChild) children;
>>>       QLIST_ENTRY(BusState) sibling;
>>> };
>>>
>>> Add the following lines of code to the add/remove child functions in
>>> hw/core/qdev.c:
>>>
>>> static void bus_add_child(BusState *bus, DeviceState *child)
>>> {
>>>       char name[32];
>>>       BusChild *kid = g_malloc0(sizeof(*kid));
>>>
>>>       kid->index = bus->max_index++;
>>>       kid->child = child;
>>>       object_ref(OBJECT(kid->child));
>>>
>>>       QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
>>>
>>>       /* This transfers ownership of kid->child to the property.  */
>>>       snprintf(name, sizeof(name), "child[%d]", kid->index);
>>>       object_property_add_link(OBJECT(bus), name,
>>>                                object_get_typename(OBJECT(child)),
>>>                                (Object **)&kid->child,
>>>                                NULL, /* read-only property */
>>>                                0, /* return ownership on prop deletion */
>>>                                NULL);
>>>
>>> +    bus->num_children++;
>>> }
>>>
>>> static void bus_remove_child(BusState *bus, DeviceState *child)
>>> {
>>>       BusChild *kid;
>>>
>>>       QTAILQ_FOREACH(kid, &bus->children, sibling) {
>>>           if (kid->child == child) {
>>>               char name[32];
>>>
>>>               snprintf(name, sizeof(name), "child[%d]", kid->index);
>>>               QTAILQ_REMOVE(&bus->children, kid, sibling);
>>>
>>>               /* This gives back ownership of kid->child back to us.  */
>>>               object_property_del(OBJECT(bus), name, NULL);
>>>               object_unref(OBJECT(kid->child));
>>>               g_free(kid);
>>>
>>> +           bus->num_children--;
>>>
>>>               return;
>>>           }
>>>       }
>>> }
>>>
>>> Change the line of code in the qbus_is_full() function in
>>> qdev_monitor.c:
>>>
>>>
>>> static inline bool qbus_is_full(BusState *bus)
>>> {
>>>       BusClass *bus_class = BUS_GET_CLASS(bus);
>>> -    return bus_class->max_dev &&
>>> -           bus->max_index >= bus_class->max_dev;
>>> +    return bus_class->max_dev &&
>>> +           bus->num_children >= bus_class->max_dev;
>>> }
>>>
>>
>> looks good to me
>> [...]
>>
> 
> I agree, the second proposal looks reasonable. Can you send a proper
> patch so I can r-b it?

will do


> 
> Halil
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-12-13 16:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-10 19:14 [Qemu-devel] [PATCH] qdev/core: Can not replug device on bus that allows one device Tony Krowiak
2018-12-11 15:23 ` Igor Mammedov
2018-12-11 19:41   ` Tony Krowiak
2018-12-13 12:09     ` Igor Mammedov
2018-12-13 13:03       ` Halil Pasic
2018-12-13 16:10         ` Tony Krowiak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).