| Message ID | 1386085320-12965-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 4, 2013 at 1:42 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > From: Hervé Poussineau <hpoussin@reactos.org> > > We should not modify the type hash table while it is being iterated on. > Assert that it does not happen. > > Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > qom/object.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/qom/object.c b/qom/object.c > index 3a43186..1dee9f0 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -78,8 +78,10 @@ static GHashTable *type_table_get(void) > return type_table; > } > > +static bool enumerating = false; Global variable could probably use a more descriptive name. Regards, Peter > static void type_table_add(TypeImpl *ti) > { > + assert(!enumerating); > g_hash_table_insert(type_table_get(), (void *)ti->name, ti); > } > > @@ -666,7 +668,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), > { > OCFData data = { fn, implements_type, include_abstract, opaque }; > > + enumerating = true; > g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, &data); > + enumerating = false; > } > > int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), > -- > 1.8.4.2 > >
Am 04.12.2013 06:51, schrieb Peter Crosthwaite: > On Wed, Dec 4, 2013 at 1:42 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> From: Hervé Poussineau <hpoussin@reactos.org> >> >> We should not modify the type hash table while it is being iterated on. >> Assert that it does not happen. >> >> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> qom/object.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/qom/object.c b/qom/object.c >> index 3a43186..1dee9f0 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -78,8 +78,10 @@ static GHashTable *type_table_get(void) >> return type_table; >> } >> >> +static bool enumerating = false; > > Global variable could probably use a more descriptive name. I renamed it to enumerating_types and dropped the assignment as suggested elsewhere by Alexey (a reply here would've been nice!). I also took the liberty of inserted a white line to make the function better readable. Regards, Andreas
On 12/16/2013 08:23 AM, Andreas Färber wrote: > Am 04.12.2013 06:51, schrieb Peter Crosthwaite: >> On Wed, Dec 4, 2013 at 1:42 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> From: Hervé Poussineau <hpoussin@reactos.org> >>> >>> We should not modify the type hash table while it is being iterated on. >>> Assert that it does not happen. >>> >>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> --- >>> qom/object.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/qom/object.c b/qom/object.c >>> index 3a43186..1dee9f0 100644 >>> --- a/qom/object.c >>> +++ b/qom/object.c >>> @@ -78,8 +78,10 @@ static GHashTable *type_table_get(void) >>> return type_table; >>> } >>> >>> +static bool enumerating = false; >> >> Global variable could probably use a more descriptive name. > > I renamed it to enumerating_types and dropped the assignment as > suggested elsewhere by Alexey (a reply here would've been nice!). Whose reply? To what? :) > > I also took the liberty of inserted a white line to make the function > better readable. I do not mind, this was not my stuff :) What I wonder about is what is going to happen to the rest of what I posted? Should I wait till this qom-next gets merged to upstream and repost my patches for Alex Graf again? Thanks.
Am 19.12.2013 12:38, schrieb Alexey Kardashevskiy: > On 12/16/2013 08:23 AM, Andreas Färber wrote: >> Am 04.12.2013 06:51, schrieb Peter Crosthwaite: >>> On Wed, Dec 4, 2013 at 1:42 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >>>> From: Hervé Poussineau <hpoussin@reactos.org> >>>> >>>> We should not modify the type hash table while it is being iterated on. >>>> Assert that it does not happen. >>>> >>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>>> --- >>>> qom/object.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/qom/object.c b/qom/object.c >>>> index 3a43186..1dee9f0 100644 >>>> --- a/qom/object.c >>>> +++ b/qom/object.c >>>> @@ -78,8 +78,10 @@ static GHashTable *type_table_get(void) >>>> return type_table; >>>> } >>>> >>>> +static bool enumerating = false; >>> >>> Global variable could probably use a more descriptive name. >> >> I renamed it to enumerating_types and dropped the assignment as >> suggested elsewhere by Alexey (a reply here would've been nice!). > > Whose reply? To what? :) A reply of yours to Peter C.'s change request, stating that you have addressed it - and in this case in which series. :) >> I also took the liberty of inserted a white line to make the function >> better readable. > > I do not mind, this was not my stuff :) > > What I wonder about is what is going to happen to the rest of what I > posted? Should I wait till this qom-next gets merged to upstream and repost > my patches for Alex Graf again? Thanks. I do intend to post a pull before Chistmas, reviewing+applying the last no_user patch from Markus was one key ingredient for that. Not sure which rest exactly you're referring to? I need to review your latest QemuOpts proposal among others. Actually I have a whole A4 page written down with series I'm wading through. ;) Cheers, Andreas
On 12/20/2013 10:29 PM, Andreas Färber wrote: > Am 19.12.2013 12:38, schrieb Alexey Kardashevskiy: >> On 12/16/2013 08:23 AM, Andreas Färber wrote: >>> Am 04.12.2013 06:51, schrieb Peter Crosthwaite: >>>> On Wed, Dec 4, 2013 at 1:42 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >>>>> From: Hervé Poussineau <hpoussin@reactos.org> >>>>> >>>>> We should not modify the type hash table while it is being iterated on. >>>>> Assert that it does not happen. >>>>> >>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>>>> --- >>>>> qom/object.c | 4 ++++ >>>>> 1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/qom/object.c b/qom/object.c >>>>> index 3a43186..1dee9f0 100644 >>>>> --- a/qom/object.c >>>>> +++ b/qom/object.c >>>>> @@ -78,8 +78,10 @@ static GHashTable *type_table_get(void) >>>>> return type_table; >>>>> } >>>>> >>>>> +static bool enumerating = false; >>>> >>>> Global variable could probably use a more descriptive name. >>> >>> I renamed it to enumerating_types and dropped the assignment as >>> suggested elsewhere by Alexey (a reply here would've been nice!). >> >> Whose reply? To what? :) > > A reply of yours to Peter C.'s change request, stating that you have > addressed it - and in this case in which series. :) You lost me here. I got these patches from Paolo Bonzini and I was told to repost them together with my other patches, that's it :) >>> I also took the liberty of inserted a white line to make the function >>> better readable. >> >> I do not mind, this was not my stuff :) >> >> What I wonder about is what is going to happen to the rest of what I >> posted? Should I wait till this qom-next gets merged to upstream and repost >> my patches for Alex Graf again? Thanks. > > I do intend to post a pull before Chistmas, reviewing+applying the last > no_user patch from Markus was one key ingredient for that. > > Not sure which rest exactly you're referring to? I need to review your > latest QemuOpts proposal among others. Actually I have a whole A4 page > written down with series I'm wading through. ;) I posted the "[PATCH v4 0/8] spapr: bootindex support" series which included the 2 patches you mentioned in this series, from the rest - 3 are for spapr and 3 are platform independent.
On 12/20/2013 10:43 PM, Alexey Kardashevskiy wrote: > On 12/20/2013 10:29 PM, Andreas Färber wrote: >> Am 19.12.2013 12:38, schrieb Alexey Kardashevskiy: >>> On 12/16/2013 08:23 AM, Andreas Färber wrote: >>>> Am 04.12.2013 06:51, schrieb Peter Crosthwaite: >>>>> On Wed, Dec 4, 2013 at 1:42 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >>>>>> From: Hervé Poussineau <hpoussin@reactos.org> >>>>>> >>>>>> We should not modify the type hash table while it is being iterated on. >>>>>> Assert that it does not happen. >>>>>> >>>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >>>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>>>>> --- >>>>>> qom/object.c | 4 ++++ >>>>>> 1 file changed, 4 insertions(+) >>>>>> >>>>>> diff --git a/qom/object.c b/qom/object.c >>>>>> index 3a43186..1dee9f0 100644 >>>>>> --- a/qom/object.c >>>>>> +++ b/qom/object.c >>>>>> @@ -78,8 +78,10 @@ static GHashTable *type_table_get(void) >>>>>> return type_table; >>>>>> } >>>>>> >>>>>> +static bool enumerating = false; >>>>> >>>>> Global variable could probably use a more descriptive name. >>>> >>>> I renamed it to enumerating_types and dropped the assignment as >>>> suggested elsewhere by Alexey (a reply here would've been nice!). >>> >>> Whose reply? To what? :) >> >> A reply of yours to Peter C.'s change request, stating that you have >> addressed it - and in this case in which series. :) > > You lost me here. I got these patches from Paolo Bonzini and I was told to > repost them together with my other patches, that's it :) > > >>>> I also took the liberty of inserted a white line to make the function >>>> better readable. >>> >>> I do not mind, this was not my stuff :) >>> >>> What I wonder about is what is going to happen to the rest of what I >>> posted? Should I wait till this qom-next gets merged to upstream and repost >>> my patches for Alex Graf again? Thanks. >> >> I do intend to post a pull before Chistmas, reviewing+applying the last >> no_user patch from Markus was one key ingredient for that. >> >> Not sure which rest exactly you're referring to? I need to review your >> latest QemuOpts proposal among others. Actually I have a whole A4 page >> written down with series I'm wading through. ;) > > > I posted the "[PATCH v4 0/8] spapr: bootindex support" series which > included the 2 patches you mentioned in this series, from the rest - 3 are > for spapr and 3 are platform independent. So? What do I do with my 6 patches from the "[PATCH v4 0/8] spapr: bootindex support" series? Thanks!
Am 24.12.2013 07:49, schrieb Alexey Kardashevskiy: > On 12/20/2013 10:43 PM, Alexey Kardashevskiy wrote: >> On 12/20/2013 10:29 PM, Andreas Färber wrote: >>> Am 19.12.2013 12:38, schrieb Alexey Kardashevskiy: >>>> What I wonder about is what is going to happen to the rest of what I >>>> posted? Should I wait till this qom-next gets merged to upstream and repost >>>> my patches for Alex Graf again? Thanks. >>> >>> I do intend to post a pull before Chistmas, reviewing+applying the last >>> no_user patch from Markus was one key ingredient for that. >>> >>> Not sure which rest exactly you're referring to? I need to review your >>> latest QemuOpts proposal among others. Actually I have a whole A4 page >>> written down with series I'm wading through. ;) >> >> >> I posted the "[PATCH v4 0/8] spapr: bootindex support" series which >> included the 2 patches you mentioned in this series, from the rest - 3 are >> for spapr and 3 are platform independent. > > > > So? What do I do with my 6 patches from the "[PATCH v4 0/8] spapr: > bootindex support" series? Thanks! Be patient. ;) 1) I am on holidays and thus not available full-time. 2) Alex has already sent out his ppc PULL, so no urgency. 3) As mentioned, I plan to post QOM and CPU PULLs later today. And no, you don't need to repost after parts of your series get applied pretty much unmodified. Andreas
diff --git a/qom/object.c b/qom/object.c index 3a43186..1dee9f0 100644 --- a/qom/object.c +++ b/qom/object.c @@ -78,8 +78,10 @@ static GHashTable *type_table_get(void) return type_table; } +static bool enumerating = false; static void type_table_add(TypeImpl *ti) { + assert(!enumerating); g_hash_table_insert(type_table_get(), (void *)ti->name, ti); } @@ -666,7 +668,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), { OCFData data = { fn, implements_type, include_abstract, opaque }; + enumerating = true; g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, &data); + enumerating = false; } int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque),