It will come handy in the near future when we will filter some capabilities based on it. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_migration.c | 3 ++- src/qemu/qemu_process.c | 9 ++++++--- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a04095e..3992b2b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3583,7 +3583,9 @@ virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary) virQEMUCapsPtr -virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, const char *binary) +virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, + const char *binary, + const char *machineType ATTRIBUTE_UNUSED) { virQEMUCapsPtr qemuCaps = virQEMUCapsCacheLookup(cache, binary); virQEMUCapsPtr ret; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 1c1227a..242a33d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -291,7 +291,8 @@ virQEMUCapsCachePtr virQEMUCapsCacheNew(const char *libDir, virQEMUCapsPtr virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary); virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, - const char *binary); + const char *binary, + const char *machineType); virQEMUCapsPtr virQEMUCapsCacheLookupByArch(virQEMUCapsCachePtr cache, virArch arch); void virQEMUCapsCacheFree(virQEMUCapsCachePtr cache); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4506d87..8309b8f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2918,7 +2918,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, hostIPv6Capable = true; } if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, - (*def)->emulator))) + (*def)->emulator, + (*def)->os.machine))) goto cleanup; qemuIPv6Capable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 43a64a1..49d6bf3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3828,7 +3828,8 @@ qemuProcessReconnect(void *opaque) */ if (!priv->qemuCaps && !(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, - obj->def->emulator))) + obj->def->emulator, + obj->def->os.machine))) goto error; /* In case the domain shutdown while we were not running, @@ -4460,7 +4461,8 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Determining emulator version"); virObjectUnref(priv->qemuCaps); if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, - vm->def->emulator))) + vm->def->emulator, + vm->def->os.machine))) goto cleanup; /* network devices must be "prepared" before hostdevs, because @@ -5498,7 +5500,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_DEBUG("Determining emulator version"); virObjectUnref(priv->qemuCaps); if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache, - vm->def->emulator))) + vm->def->emulator, + vm->def->os.machine))) goto error; VIR_DEBUG("Preparing monitor state"); -- 2.0.5

Not all machine types support all devices, device properties, backends, etc. So until we create a matrix of [machineType, qemuCaps], lets just filter out some capabilities before we return them to the consumer (which is going to make decisions based on them straight away). Currently, as qemu is unable to tell which capabilities are (not) enabled for given machine types, it's us who has to hardcode the matrix. One day maybe the hardcoding will go away and we can create the matrix dynamically on the fly based on a few monitor calls. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 39 ++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 2 ++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3992b2b..233449b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3511,6 +3511,42 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps) } +struct virQEMUCapsMachineTypeFilter { + const char *machineType; + virQEMUCapsFlags *flags; + size_t nflags; +}; + +static const struct virQEMUCapsMachineTypeFilter virQEMUCapsMachineFilter[] = { + /* { "blah", virQEMUCapsMachineBLAHFilter, + ARRAY_CARDINALITY(virQEMUCapsMachineBLAHFilter) }, */ + { "", NULL, 0 }, +}; + + +void +virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, + const char *machineType) +{ + size_t i; + + if (!machineType) + return; + + for (i = 0; i < ARRAY_CARDINALITY(virQEMUCapsMachineFilter); i++) { + const struct virQEMUCapsMachineTypeFilter *filter = &virQEMUCapsMachineFilter[i]; + size_t j; + + if (STRNEQ(filter->machineType, machineType)) + continue; + + for (j = 0; j < filter->nflags; j++) + virQEMUCapsClear(qemuCaps, filter->flags[j]); + } + +} + + virQEMUCapsCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, @@ -3585,7 +3621,7 @@ virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary) virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, const char *binary, - const char *machineType ATTRIBUTE_UNUSED) + const char *machineType) { virQEMUCapsPtr qemuCaps = virQEMUCapsCacheLookup(cache, binary); virQEMUCapsPtr ret; @@ -3595,6 +3631,7 @@ virQEMUCapsCacheLookupCopy(virQEMUCapsCachePtr cache, ret = virQEMUCapsNewCopy(qemuCaps); virObjectUnref(qemuCaps); + virQEMUCapsFilterByMachineType(ret, machineType); return ret; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 242a33d..5d87c18 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -284,6 +284,8 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps); +void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, + const char *machineType); virQEMUCapsCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 316f479..f2120dd 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -308,6 +308,8 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } + virQEMUCapsFilterByMachineType(extraFlags, vmdef->os.machine); + if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DEVICE)) { if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL)) { if (flags & FLAG_EXPECT_ERROR) -- 2.0.5

Well, after [1] qemu doesn't understand '-object memory-backend-ram' nor '-object memory-backend-file'. Make sure we remove that capabilities from our internal list temporarily, so the qemu command line is constructed in correct way. 1: https://bugzilla.redhat.com/show_bug.cgi?id=1170093 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 10 ++++--- .../qemuxml2argv-numatune-memnode-rhel650.args | 7 +++++ .../qemuxml2argv-numatune-memnode-rhel650.xml | 31 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ 4 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 233449b..940f070 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3510,6 +3510,11 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps) return sb.st_ctime == qemuCaps->ctime; } +static virQEMUCapsFlags virQEMUCapsMachineRHEL650Filter[] = { + /* For some reason, rhel6.5.0 machine type doesn't understand memdev. */ + QEMU_CAPS_OBJECT_MEMORY_RAM, + QEMU_CAPS_OBJECT_MEMORY_FILE, +}; struct virQEMUCapsMachineTypeFilter { const char *machineType; @@ -3518,9 +3523,8 @@ struct virQEMUCapsMachineTypeFilter { }; static const struct virQEMUCapsMachineTypeFilter virQEMUCapsMachineFilter[] = { - /* { "blah", virQEMUCapsMachineBLAHFilter, - ARRAY_CARDINALITY(virQEMUCapsMachineBLAHFilter) }, */ - { "", NULL, 0 }, + { "rhel6.5.0", virQEMUCapsMachineRHEL650Filter, + ARRAY_CARDINALITY(virQEMUCapsMachineRHEL650Filter)}, }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.args new file mode 100644 index 0000000..813f3ea --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/kvm -S -M rhel6.5.0 -m 24104 -smp 32 \ +-numa node,nodeid=0,cpus=0,mem=20 \ +-numa node,nodeid=1,cpus=1-27,cpus=29,mem=645 \ +-numa node,nodeid=2,cpus=28,cpus=30-31,mem=23440 \ +-nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.xml b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.xml new file mode 100644 index 0000000..a659216 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>QEMUGuest</name> + <uuid>9f4b6512-e73a-4a25-93e8-5307802821ce</uuid> + <memory unit='KiB'>24682468</memory> + <currentMemory unit='KiB'>24682468</currentMemory> + <vcpu placement='static'>32</vcpu> + <numatune> + <memory mode='strict' nodeset='0-7'/> + </numatune> + <os> + <type arch='x86_64' machine='rhel6.5.0'>hvm</type> + <boot dev='hd'/> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='20002' unit='KiB'/> + <cell id='1' cpus='1-27,29' memory='660066' unit='KiB'/> + <cell id='2' cpus='28-31,^29' memory='24002400' unit='KiB'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/kvm</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f2120dd..a7e14f0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1263,6 +1263,10 @@ mymain(void) DO_TEST_LINUX("numatune-memnode", QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM); DO_TEST_FAILURE("numatune-memnode", NONE); + DO_TEST_LINUX("numatune-memnode-rhel650", + QEMU_CAPS_NUMA, + QEMU_CAPS_OBJECT_MEMORY_RAM, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST_LINUX("numatune-memnode-no-memory", QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM); -- 2.0.5

On Fri, Feb 13, 2015 at 02:29:49PM +0000, Daniel P. Berrange wrote: I'm a bit hesitating on this one. Both have pros and cons. I'd vote for upstream libvirt working everywhere it can and since the difference here is not a feature difference, but a bug and the machine type says clearly "RHEL", it seems like a better option to have it upstream as well. Any concerns as to why this should be downstream only?

On 20.02.2015 12:11, Daniel P. Berrange wrote: Okay, so just before I push the former two patches, a quick confirmation. Does it make sense to push just 1/3 and 2/3? I'd argue that yes, as it gives distros opportunity to invent their own filters based on how they build qemu. Michal