[libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC

[Osier Yang]

On 2012年04月17日 04:17, Eric Blake wrote:
> [adding direct cc to Osier]
>
> On 04/16/2012 01:58 PM, Cole Robinson wrote:
>>>>> Rebasing to HEAD made things rather worse... now 3 checks are failing (without
>>>>> my patch)...
>>>>>
>>>>
>>>> Might be something simple like missing a build dep. Try getting some debug
>>>> output from the tests and it might be clear. If it isn't something simple, I
>>>> can give your patch a spin.
>>>>
>>>> - Cole
>>>
>>> So the initial failure turned out to be a missing libsasl2-dev. The other two
>>> are new after the rebase:
>>>
>>> TEST: xml2vmxtest
>>> ...
>>> 31) VMware XML-2-VMX esx-in-the-wild-2 ->  esx-in-the-wild-2
>>> @@ -7,7 +7,6 @@
>>>   memsize = "1000"
>>>   sched.mem.max = "118"
>>>   numvcpus = "4"
>>> -sched.cpu.affinity = "0,1,2,5,6,7"
>>>   scsi0.present = "true"
>>>   scsi0.virtualDev = "lsilogic"
>>>   scsi1.present = "true"
>>> 32) VMware XML-2-VMX esx-in-the-wild-3 ->  esx-in-the-wild-3
>>> @@ -6,7 +6,6 @@
>>>   displayName = "virtDebian2"
>>>   memsize = "1024"
>>>   numvcpus = "2"
>>> -sched.cpu.affinity = "0,3,4,5"
>>>   scsi0.present = "true"
>>>   scsi0.virtualDev = "lsilogic"
>>>   scsi0:0.present = "true"
>>> ...
>>> PASS: test_conf.sh
>>> --- exp 2012-04-16 17:27:09.672832070 +0000
>>> +++ out 2012-04-16 17:27:09.672832070 +0000
>>> @@ -1,3 +1,3 @@
>>>   error: Failed to define domain from xml-invalid
>>> -error: internal error topology cpuset syntax error
>>> +error: operation failed: domain 'test' already exists with uuid
>>> 96b2bf5e-ea49-17b7-ad2c-14308ca3ff59
>>> FAIL: cpuset
>>>
>>> All three go away when I revert the following patch:
>>>
>>> commit 8fb2164cfff35ce0b87f1d513a0f3ca5111d7880
>>> Author: Osier Yang<jyang at redhat.com>
>>> Date:   Wed Apr 11 22:40:33 2012 +0800
>>>
>>>      numad: Ignore cpuset if placement is auto
>
> Yes, I independently hit the same problem and conclusion this morning.
>
>>>
>>> So I would think I can call my patch tested now. ;)
>>>
>>
>> Cool, thanks for checking. ACK to the original patch.
>>
>> Osier, looks like that patch caused some test fallout?
>

Oops, yes.

> Daniel and I were discussing it on IRC, although I haven't yet tried
> patching anything:
>
>
> [09:30]	danpb1	eblake: (04:03:19 PM) eblake: +error: operation failed:
> domain 'test' already exists with uuid 88cb9633-8015-4624-a258-2a3cb9700ad0
> [09:31]	danpb1	eblake: the fact that we see that error message suggest a
> pre-existing bug in the test suite - the XML being used to define the
> test domain is missing a UUID
> [09:31]	danpb1	eblake: so the 2 repeated defines get different
> auto-generated UUIDs&  thus violate the uniqueness check
> [09:31]	danpb1	eblake: i expect if you fixed that problem, then you'd
> now see the 'define' operation actually suceed

Not true, there is UUID in the dumped xml:

# for i in {1..3}; do ./tools/virsh --connect test:///default \
   dumpxml test | grep uuid; done
   <uuid>a1b6ee1f-97de-f0ee-617a-0cdb74947df5</uuid>
   <uuid>ee68d7d2-3eb9-593e-2769-797ce1f4c4aa</uuid>
   <uuid>fecb1d3a-918a-8412-e534-76192cf32b18</uuid>

It outputs different UUID each time. I'm going to fix this first.

> [09:32]	eblake	but since the test expects a topology syntax error, what
> changed to make it no longer an error?
> [09:33]	eblake	and thus exposing the latent bug in the uuid usage
> [09:33]	danpb1	oh sure, it sounds like we still have a bug in changed
> semantics
> [09:34]	danpb1	eblake: + if (def->placement_mode ==
> VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC) {
> [09:34]	danpb1	that line means we now don't ever parse the placement XML
> and so won't see the bogus data
> [09:35]	danpb1	since the default value is PLACEMENT_MODE_DEFAULT instead
> of PLACEMENT_MODE_STATIC
> [09:35]	danpb1	we need to treat DEFAULT + STATIC in the same way in the
> parser
> [09:36]	eblake	DEFAULT is primarily useful for where there is a
> difference in the XML between omitting something and explicitly
> requesting STATIC
> [09:36]	eblake	but if we want to always default to STATIC, maybe we
> could just trim the enum to get rid of DEFAULT and make STATIC == 0

> [09:37]	danpb1	i guess the intent was that we don't suddenly fill all
> existing XML with a new attribute

Yes, that was the intent, "static" when "cpuset" is specified manually,
and "default" for "cpuset" is not specified.

> [09:38]	eblake	in which case, default (for omitted) vs. static
> (explicit, don't lose what the user had) makes sense
> [09:38]	eblake	but then I agree that we need to handle both modes the
> same when deciding what else to parse

I will post a patch soon after the uuid problem of test driver,
to treat "default" and "static" the same (not to parse cpuset).

Regards,
Osier