On 04/22/2013 10:37 PM, Laine Stump wrote:
> On 04/22/2013 02:43 PM, Ján Tomko wrote:
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index a7aabdf..ab99538 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -673,6 +673,37 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>> !(def->emulator = virDomainDefGetDefaultEmulator(def, caps)))
>> return -1;
>>
>> + /* Add implicit PCI root controller if the machine has one */
>> + switch (def->os.arch) {
>> + case VIR_ARCH_I686:
>> + case VIR_ARCH_X86_64:
>> + if (!def->os.machine)
>> + break;
>> + if (STRPREFIX(def->os.machine, "pc-q35") ||
>> + STREQ(def->os.machine, "q35") ||
>> + STREQ(def->os.machine, "isapc"))
>> + break;
>> + if (!STRPREFIX(def->os.machine, "pc-0.") &&
>> + !STRPREFIX(def->os.machine, "pc-1.") &&
>> + !STREQ(def->os.machine, "pc") &&
>> + !STRPREFIX(def->os.machine, "rhel"))
>> + break;
>
>
> If you're going to fall through to a different case like this, you
> should put a comment in the code something like this:
>
> /* FALL THROUGH TO NEXT CASE */
>
> just so people don't have to think too hard :-)
>
> However, I think it would be more easily expandable in the future if you
> had a straight switch statement with all the cases, and just set a
> "needsPCIRoot" boolean for those cases that need it, then an "if
> (needsPCIRoot)" at the end. In the future when we want to add other
> implicit devices, each case can be a mix of the appropriate "needsThis"
> and "needsThat", with the actual additions at the end.
>
>
>...
>
> ACK, with the addition of the "FALLTHROUGH" comment, or restructuring it
> is as I suggested.
I'm squashing this in before pushing:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ab99538..98ac56f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -668,6 +668,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
void *opaque ATTRIBUTE_UNUSED)
{
+ bool addPCIRoot = false;
+
/* check for emulator and create a default one if needed */
if (!def->emulator &&
!(def->emulator = virDomainDefGetDefaultEmulator(def, caps)))
@@ -688,6 +690,8 @@ qemuDomainDefPostParse(virDomainDefPtr def,
!STREQ(def->os.machine, "pc") &&
!STRPREFIX(def->os.machine, "rhel"))
break;
+ addPCIRoot = true;
+ break;
case VIR_ARCH_ALPHA:
case VIR_ARCH_PPC:
@@ -695,15 +699,18 @@ qemuDomainDefPostParse(virDomainDefPtr def,
case VIR_ARCH_PPCEMB:
case VIR_ARCH_SH4:
case VIR_ARCH_SH4EB:
- if (virDomainDefMaybeAddController(
- def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
- VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
- return -1;
+ addPCIRoot = true;
break;
default:
break;
}
+ if (addPCIRoot &&
+ virDomainDefMaybeAddController(
+ def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
+ return -1;
+
return 0;
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list