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

[Daniel P. Berrange]

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 at 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 :|