[libvirt] [PATCH 0/3] Introduce machine type based capabilities filtering
Well, here's an alternative approach to my question here [1]. 1: https://www.redhat.com/archives/libvir-list/2015-February/msg00369.html Michal Privoznik (3): virQEMUCapsCacheLookupCopy: Pass machine type virQEMUCapsCacheLookupCopy: Filter qemuCaps based on machineType qemuCaps: Disable memdev for rhel6.5.0 machine type src/qemu/qemu_capabilities.c | 45 +++++++++++++++++++++- src/qemu/qemu_capabilities.h | 5 ++- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_process.c | 9 +++-- .../qemuxml2argv-numatune-memnode-rhel650.args | 7 ++++ .../qemuxml2argv-numatune-memnode-rhel650.xml | 31 +++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 7 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-rhel650.xml -- 2.0.5
It will come handy in the near future when we will filter some capabilities based on it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- 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
On Thu, Feb 12, 2015 at 04:09:38PM +0100, Michal Privoznik wrote:
It will come handy in the near future when we will filter some capabilities based on it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- 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(-)
ACK
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@redhat.com> --- 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
On Thu, Feb 12, 2015 at 04:09:39PM +0100, Michal Privoznik wrote:
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@redhat.com> --- src/qemu/qemu_capabilities.c | 39 ++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 2 ++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 42 insertions(+), 1 deletion(-)
ACK
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@redhat.com> --- 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 Thu, Feb 12, 2015 at 04:09:40PM +0100, Michal Privoznik wrote:
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@redhat.com> --- 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)}, };
FWIW, I'd consider this to be something that RHEL downstream RPMs should carry, not for upstream. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Feb 13, 2015 at 02:29:49PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 12, 2015 at 04:09:40PM +0100, Michal Privoznik wrote:
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@redhat.com> --- 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)}, };
FWIW, I'd consider this to be something that RHEL downstream RPMs should carry, not for upstream.
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?
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Feb 20, 2015 at 12:01:58PM +0100, Martin Kletzander wrote:
On Fri, Feb 13, 2015 at 02:29:49PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 12, 2015 at 04:09:40PM +0100, Michal Privoznik wrote:
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@redhat.com> --- 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)}, };
FWIW, I'd consider this to be something that RHEL downstream RPMs should carry, not for upstream.
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?
If we accept RHEL specific hacks, then it follows that we should accept hacks for SUSE, Ubuntu, *BSD, etc to work around whatever mistakes they make in their distros too. I don't it is a scalable path for libvirt upstream to take responsibility for fixing every distros' mistakes and don't think RHEL should get a special free pass just because alot of Red Hat people work on libvirt. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 20.02.2015 12:11, Daniel P. Berrange wrote:
On Fri, Feb 20, 2015 at 12:01:58PM +0100, Martin Kletzander wrote:
On Fri, Feb 13, 2015 at 02:29:49PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 12, 2015 at 04:09:40PM +0100, Michal Privoznik wrote:
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@redhat.com> --- 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)}, };
FWIW, I'd consider this to be something that RHEL downstream RPMs should carry, not for upstream.
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?
If we accept RHEL specific hacks, then it follows that we should accept hacks for SUSE, Ubuntu, *BSD, etc to work around whatever mistakes they make in their distros too. I don't it is a scalable path for libvirt upstream to take responsibility for fixing every distros' mistakes and don't think RHEL should get a special free pass just because alot of Red Hat people work on libvirt.
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
On Fri, Feb 20, 2015 at 01:35:17PM +0100, Michal Privoznik wrote:
On 20.02.2015 12:11, Daniel P. Berrange wrote:
On Fri, Feb 20, 2015 at 12:01:58PM +0100, Martin Kletzander wrote:
On Fri, Feb 13, 2015 at 02:29:49PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 12, 2015 at 04:09:40PM +0100, Michal Privoznik wrote:
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@redhat.com> --- 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)}, };
FWIW, I'd consider this to be something that RHEL downstream RPMs should carry, not for upstream.
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?
If we accept RHEL specific hacks, then it follows that we should accept hacks for SUSE, Ubuntu, *BSD, etc to work around whatever mistakes they make in their distros too. I don't it is a scalable path for libvirt upstream to take responsibility for fixing every distros' mistakes and don't think RHEL should get a special free pass just because alot of Red Hat people work on libvirt.
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.
I can see us needing todo machine type specific checks for different architectures in the future, so 1 & 2 are fine with me Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Martin Kletzander -
Michal Privoznik