I'm not familiar with the libvirt test suite but let me see if I can enhance it a little bit. Best Regards Yudai Yamagishi 2013/12/17 Jiri Denemark <jdenemar@xxxxxxxxxx>: > On Tue, Dec 17, 2013 at 17:33:19 +0900, Yudai Yamagishi wrote: >> From: Yudai Yamagish <yummy@xxxxxxxxxxxxxx> >> >> This patch fixes a segmentation fault when creating new virtual machines using QEMU. >> The segmentation fault is caused by commit f41830680e40d3ec845cefd25419bd87414b9ccf >> and commit cbb6ec42e2447d7920b30d66923b2a2b2670133b. >> >> In virQEMUCapsProbeQMPMachineTypes, when copying machines to qemuCaps, "none" is skipped. >> Therefore, the value of i and "qemuCaps->nmachineTypes - 1" do not always match. >> However, defIdx value (used to call virQEMUCapsSetDefaultMachine) is set using the value in i >> when the array elements are in qemuCaps->nmachineTypes - 1. >> So, when libvirt tries to create virtual machines using the default machine type, >> qemuCaps->machineTypes[defIdx] is accessed and since the defIdx is NULL, it results in segmentation fault. >> >> Signed-off-by: Yudai Yamagishi <yummy@xxxxxxxxxxxxxx> >> --- >> src/qemu/qemu_capabilities.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c >> index 5e9c65e..5def55c 100644 >> --- a/src/qemu/qemu_capabilities.c >> +++ b/src/qemu/qemu_capabilities.c >> @@ -2150,7 +2150,7 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, >> machines[i]->name) < 0) >> goto cleanup; >> if (machines[i]->isDefault) >> - defIdx = i; >> + defIdx = qemuCaps->nmachineTypes - 1; >> qemuCaps->machineMaxCpus[qemuCaps->nmachineTypes - 1] = >> machines[i]->maxCpus; >> } > > Oops, good catch. The reason we haven't seen it yet is the order of > machine types returned by your QEMU is apparently different than the > order in which anyone else gets them :-) If "none" comes after the > default machine type, everything works as expected. Also if the default > machine is not the last one, libvirt would just select a wrong machine > type as the default one instead of segfaulting. > > Yudai, would you be comfortable with updating our test suite to test > this corner case? We already have qemucapabilitiestest.c but it only > tests QEMU capabilities even though the data files already contain > machine types. So the test would need to be enhanced to actually check > machine types decoded from the data files. And we would need a data file > where "none" is not the last machine type in the list. > > Jirka -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list