diff mbox

[PULL,05/23] pc-dimm: add pc_dimm_build_list()

Message ID 56CEC19C.4020400@virtuozzo.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy Feb. 25, 2016, 8:55 a.m. UTC
On 25.02.2016 11:39, Michael S. Tsirkin wrote:
> On Thu, Feb 25, 2016 at 10:01:18AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Stop!
> It builds fine here. Did you check out my tree?

anyway, this file should be here, as it is added into Makefile:



===========
git clone git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tmp
cd tmp/
git checkout for_upstream
./configure  --target-list=x86_64-softmmu
make -j9
 > make: *** No rule to make target `stubs/pc_dimm.o', needed by 
`libqemustub.a'.  Stop

>
>> Hey, stubs/pc_dimm.c disappeared from this patch:
>> =========== from [PATCH v7 0/5] don't use NVDIMM for balooning/[PATCH 3/5]
>> pc-dimm: add pc_dimm_build_list() ===================
>>
>> diff --git a/stubs/pc_dimm.c b/stubs/pc_dimm.c
>> new file mode 100644
>> index 0000000..5312f50
>> --- /dev/null
>> +++ b/stubs/pc_dimm.c
>> @@ -0,0 +1,12 @@
>> +#include "qom/object.h"
>> +#include "hw/mem/pc-dimm.h"
>> +
>> +int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>> +{
>> +   return 0;
>> +}
>> +
>> +int pc_dimm_build_list(Object *obj, void *opaque)
>> +{
>> +   return 0;
>> +}
>>
>> ================================================================
>>
>>
>> This will not compile!!!
>> make: *** No rule to make target `stubs/pc_dimm.c', needed by
>> `stubs/pc_dimm.o'.  Stop.
>>
>>
>> On 24.02.2016 23:35, Michael S. Tsirkin wrote:
>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>> Like pc_dimm_build_list_sorted but not sorted - for cases where sorting
>>> is not necessary. Add stubbed version too - for targets without
>>> CONFIG_MEM_HOTPLUG.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>   include/hw/mem/pc-dimm.h        |  3 +++
>>>   hw/mem/pc-dimm.c                | 15 +++++++++++++++
>>>   stubs/qmp_pc_dimm_device_list.c |  8 --------
>>>   stubs/Makefile.objs             |  2 +-
>>>   4 files changed, 19 insertions(+), 9 deletions(-)
>>>   delete mode 100644 stubs/qmp_pc_dimm_device_list.c
>>>
>>> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
>>> index 218dfb0..0f587a4 100644
>>> --- a/include/hw/mem/pc-dimm.h
>>> +++ b/include/hw/mem/pc-dimm.h
>>> @@ -94,4 +94,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
>>>                            MemoryRegion *mr, uint64_t align, Error **errp);
>>>   void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
>>>                              MemoryRegion *mr);
>>> +
>>> +int pc_dimm_build_list(Object *obj, void *opaque);
>>> +
>>>   #endif
>>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>>> index 835179e..09e99ce 100644
>>> --- a/hw/mem/pc-dimm.c
>>> +++ b/hw/mem/pc-dimm.c
>>> @@ -267,6 +267,21 @@ static int pc_dimm_build_list_sorted(Object *obj, void *opaque)
>>>       return 0;
>>>   }
>>> +int pc_dimm_build_list(Object *obj, void *opaque)
>>> +{
>>> +    GSList **list = opaque;
>>> +
>>> +    if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
>>> +        DeviceState *dev = DEVICE(obj);
>>> +        if (dev->realized) { /* only realized DIMMs matter */
>>> +            *list = g_slist_prepend(*list, dev);
>>> +        }
>>> +    }
>>> +
>>> +    object_child_foreach(obj, pc_dimm_build_list, opaque);
>>> +    return 0;
>>> +}
>>> +
>>>   uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
>>>                                  uint64_t address_space_size,
>>>                                  uint64_t *hint, uint64_t align, uint64_t size,
>>> diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm_device_list.c
>>> deleted file mode 100644
>>> index def2115..0000000
>>> --- a/stubs/qmp_pc_dimm_device_list.c
>>> +++ /dev/null
>>> @@ -1,8 +0,0 @@
>>> -#include "qemu/osdep.h"
>>> -#include "qom/object.h"
>>> -#include "hw/mem/pc-dimm.h"
>>> -
>>> -int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>>> -{
>>> -   return 0;
>>> -}
>>> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
>>> index e922de9..fb247d4 100644
>>> --- a/stubs/Makefile.objs
>>> +++ b/stubs/Makefile.objs
>>> @@ -35,7 +35,7 @@ stub-obj-y += vmstate.o
>>>   stub-obj-$(CONFIG_WIN32) += fd-register.o
>>>   stub-obj-y += cpus.o
>>>   stub-obj-y += kvm.o
>>> -stub-obj-y += qmp_pc_dimm_device_list.o
>>> +stub-obj-y += pc_dimm.o
>>>   stub-obj-y += target-monitor-defs.o
>>>   stub-obj-y += target-get-monitor-def.o
>>>   stub-obj-y += vhost.o
>>
>> -- 
>> Best regards,
>> Vladimir

Comments

Michael S. Tsirkin Feb. 25, 2016, 9:11 a.m. UTC | #1
On Thu, Feb 25, 2016 at 11:55:56AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 25.02.2016 11:39, Michael S. Tsirkin wrote:
> >On Thu, Feb 25, 2016 at 10:01:18AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> >>Stop!
> >It builds fine here. Did you check out my tree?
> 
> anyway, this file should be here, as it is added into Makefile:
> 
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -35,7 +35,7 @@ stub-obj-y += vmstate.o
>  stub-obj-$(CONFIG_WIN32) += fd-register.o
>  stub-obj-y += cpus.o
>  stub-obj-y += kvm.o
> -stub-obj-y += qmp_pc_dimm_device_list.o
> +stub-obj-y += pc_dimm.o
>  stub-obj-y += target-monitor-defs.o
>  stub-obj-y += target-get-monitor-def.o
>  stub-obj-y += vhost.o
> 

Hmm that's right.
Thanks!
Looks like it was picking up an old object in the filesystem :(
I'll respin - for now, does everything work for you if you drop
this line from the makefile?

> ===========
> git clone git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tmp
> cd tmp/
> git checkout for_upstream
> ./configure  --target-list=x86_64-softmmu
> make -j9
> > make: *** No rule to make target `stubs/pc_dimm.o', needed by
> `libqemustub.a'.  Stop
Vladimir Sementsov-Ogievskiy Feb. 25, 2016, 9:54 a.m. UTC | #2
On 25.02.2016 12:11, Michael S. Tsirkin wrote:
> On Thu, Feb 25, 2016 at 11:55:56AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> On 25.02.2016 11:39, Michael S. Tsirkin wrote:
>>> On Thu, Feb 25, 2016 at 10:01:18AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> Stop!
>>> It builds fine here. Did you check out my tree?
>> anyway, this file should be here, as it is added into Makefile:
>>
>> --- a/stubs/Makefile.objs
>> +++ b/stubs/Makefile.objs
>> @@ -35,7 +35,7 @@ stub-obj-y += vmstate.o
>>   stub-obj-$(CONFIG_WIN32) += fd-register.o
>>   stub-obj-y += cpus.o
>>   stub-obj-y += kvm.o
>> -stub-obj-y += qmp_pc_dimm_device_list.o
>> +stub-obj-y += pc_dimm.o
>>   stub-obj-y += target-monitor-defs.o
>>   stub-obj-y += target-get-monitor-def.o
>>   stub-obj-y += vhost.o
>>
> Hmm that's right.
> Thanks!
> Looks like it was picking up an old object in the filesystem :(
> I'll respin - for now, does everything work for you if you drop
> this line from the makefile?

it will break compilation for targets without memory hotplug, it was 
discussed on list. stubs are necessary.

>
>> ===========
>> git clone git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tmp
>> cd tmp/
>> git checkout for_upstream
>> ./configure  --target-list=x86_64-softmmu
>> make -j9
>>> make: *** No rule to make target `stubs/pc_dimm.o', needed by
>> `libqemustub.a'.  Stop
Paolo Bonzini Feb. 25, 2016, 10:09 a.m. UTC | #3
On 25/02/2016 10:54, Vladimir Sementsov-Ogievskiy wrote:
> On 25.02.2016 12:11, Michael S. Tsirkin wrote:
>> On Thu, Feb 25, 2016 at 11:55:56AM +0300, Vladimir Sementsov-Ogievskiy
>> wrote:
>>> On 25.02.2016 11:39, Michael S. Tsirkin wrote:
>>>> On Thu, Feb 25, 2016 at 10:01:18AM +0300, Vladimir
>>>> Sementsov-Ogievskiy wrote:
>>>>> Stop!
>>>> It builds fine here. Did you check out my tree?
>>> anyway, this file should be here, as it is added into Makefile:
>>>
>>> --- a/stubs/Makefile.objs
>>> +++ b/stubs/Makefile.objs
>>> @@ -35,7 +35,7 @@ stub-obj-y += vmstate.o
>>>   stub-obj-$(CONFIG_WIN32) += fd-register.o
>>>   stub-obj-y += cpus.o
>>>   stub-obj-y += kvm.o
>>> -stub-obj-y += qmp_pc_dimm_device_list.o
>>> +stub-obj-y += pc_dimm.o
>>>   stub-obj-y += target-monitor-defs.o
>>>   stub-obj-y += target-get-monitor-def.o
>>>   stub-obj-y += vhost.o
>>>
>> Hmm that's right.
>> Thanks!
>> Looks like it was picking up an old object in the filesystem :(
>> I'll respin - for now, does everything work for you if you drop
>> this line from the makefile?
> 
> it will break compilation for targets without memory hotplug, it was
> discussed on list. stubs are necessary.

mst,

indeed it's not clear to me why you're deleting qmp_pc_dimm_device_list.c...

On the other hand, pc_dimm_build_list doesn't need a stub.  Just put it
in hw/virtio/virtio-balloon.c.

Paolo

> 
>>
>>> ===========
>>> git clone git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tmp
>>> cd tmp/
>>> git checkout for_upstream
>>> ./configure  --target-list=x86_64-softmmu
>>> make -j9
>>>> make: *** No rule to make target `stubs/pc_dimm.o', needed by
>>> `libqemustub.a'.  Stop
> 
>
Michael S. Tsirkin Feb. 25, 2016, 10:12 a.m. UTC | #4
On Thu, Feb 25, 2016 at 11:09:16AM +0100, Paolo Bonzini wrote:
> 
> 
> On 25/02/2016 10:54, Vladimir Sementsov-Ogievskiy wrote:
> > On 25.02.2016 12:11, Michael S. Tsirkin wrote:
> >> On Thu, Feb 25, 2016 at 11:55:56AM +0300, Vladimir Sementsov-Ogievskiy
> >> wrote:
> >>> On 25.02.2016 11:39, Michael S. Tsirkin wrote:
> >>>> On Thu, Feb 25, 2016 at 10:01:18AM +0300, Vladimir
> >>>> Sementsov-Ogievskiy wrote:
> >>>>> Stop!
> >>>> It builds fine here. Did you check out my tree?
> >>> anyway, this file should be here, as it is added into Makefile:
> >>>
> >>> --- a/stubs/Makefile.objs
> >>> +++ b/stubs/Makefile.objs
> >>> @@ -35,7 +35,7 @@ stub-obj-y += vmstate.o
> >>>   stub-obj-$(CONFIG_WIN32) += fd-register.o
> >>>   stub-obj-y += cpus.o
> >>>   stub-obj-y += kvm.o
> >>> -stub-obj-y += qmp_pc_dimm_device_list.o
> >>> +stub-obj-y += pc_dimm.o
> >>>   stub-obj-y += target-monitor-defs.o
> >>>   stub-obj-y += target-get-monitor-def.o
> >>>   stub-obj-y += vhost.o
> >>>
> >> Hmm that's right.
> >> Thanks!
> >> Looks like it was picking up an old object in the filesystem :(
> >> I'll respin - for now, does everything work for you if you drop
> >> this line from the makefile?
> > 
> > it will break compilation for targets without memory hotplug, it was
> > discussed on list. stubs are necessary.
> 
> mst,
> 
> indeed it's not clear to me why you're deleting qmp_pc_dimm_device_list.c...
> 
> On the other hand, pc_dimm_build_list doesn't need a stub.  Just put it
> in hw/virtio/virtio-balloon.c.
> 
> Paolo

Bad merge and I corrupted my test system again so it was
missing the problem :(

I'm fixing it up, thanks everyone!

> > 
> >>
> >>> ===========
> >>> git clone git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tmp
> >>> cd tmp/
> >>> git checkout for_upstream
> >>> ./configure  --target-list=x86_64-softmmu
> >>> make -j9
> >>>> make: *** No rule to make target `stubs/pc_dimm.o', needed by
> >>> `libqemustub.a'.  Stop
> > 
> >
Vladimir Sementsov-Ogievskiy Feb. 26, 2016, 9:08 a.m. UTC | #5
On 25.02.2016 13:09, Paolo Bonzini wrote:
>
> On 25/02/2016 10:54, Vladimir Sementsov-Ogievskiy wrote:
>> On 25.02.2016 12:11, Michael S. Tsirkin wrote:
>>> On Thu, Feb 25, 2016 at 11:55:56AM +0300, Vladimir Sementsov-Ogievskiy
>>> wrote:
>>>> On 25.02.2016 11:39, Michael S. Tsirkin wrote:
>>>>> On Thu, Feb 25, 2016 at 10:01:18AM +0300, Vladimir
>>>>> Sementsov-Ogievskiy wrote:
>>>>>> Stop!
>>>>> It builds fine here. Did you check out my tree?
>>>> anyway, this file should be here, as it is added into Makefile:
>>>>
>>>> --- a/stubs/Makefile.objs
>>>> +++ b/stubs/Makefile.objs
>>>> @@ -35,7 +35,7 @@ stub-obj-y += vmstate.o
>>>>    stub-obj-$(CONFIG_WIN32) += fd-register.o
>>>>    stub-obj-y += cpus.o
>>>>    stub-obj-y += kvm.o
>>>> -stub-obj-y += qmp_pc_dimm_device_list.o
>>>> +stub-obj-y += pc_dimm.o
>>>>    stub-obj-y += target-monitor-defs.o
>>>>    stub-obj-y += target-get-monitor-def.o
>>>>    stub-obj-y += vhost.o
>>>>
>>> Hmm that's right.
>>> Thanks!
>>> Looks like it was picking up an old object in the filesystem :(
>>> I'll respin - for now, does everything work for you if you drop
>>> this line from the makefile?
>> it will break compilation for targets without memory hotplug, it was
>> discussed on list. stubs are necessary.
> mst,
>
> indeed it's not clear to me why you're deleting qmp_pc_dimm_device_list.c...

to put pc_dimm related stubs (qmp_pc_dimm_device_list, 
pc_dimm_build_list) into one file - stubs/pc_dimm.c

>
> On the other hand, pc_dimm_build_list doesn't need a stub.  Just put it
> in hw/virtio/virtio-balloon.c.

It's up to you ofcourse, but for me it is strange. Logically, 
pc_dimm_build_list is related to pc_dimm.c, and, also there is very 
similar function pc_dimm_build_list_sorted - it is in pc_dimm.c too (may 
be these two function should be merged somehow in future).

>
> Paolo
>
>>>> ===========
>>>> git clone git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tmp
>>>> cd tmp/
>>>> git checkout for_upstream
>>>> ./configure  --target-list=x86_64-softmmu
>>>> make -j9
>>>>> make: *** No rule to make target `stubs/pc_dimm.o', needed by
>>>> `libqemustub.a'.  Stop
>>
diff mbox

Patch

--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -35,7 +35,7 @@  stub-obj-y += vmstate.o
  stub-obj-$(CONFIG_WIN32) += fd-register.o
  stub-obj-y += cpus.o
  stub-obj-y += kvm.o
-stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += pc_dimm.o
  stub-obj-y += target-monitor-defs.o
  stub-obj-y += target-get-monitor-def.o
  stub-obj-y += vhost.o