Re: [libvirt] [PATCH 3/3] qemuCaps: Disable memdev for rhel6.5.0 machine type

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(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)}, >>>> }; >>> >>> 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.