[libvirt] [PATCH v2 11/11] conf: Add VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING

[Cole Robinson]

On 4/17/19 7:28 AM, Daniel P. Berrangé wrote:
> On Tue, Apr 16, 2019 at 12:59:23PM -0400, Cole Robinson wrote:
>> On 4/16/19 11:26 AM, Michal Privoznik wrote:
>>> On 3/13/19 4:51 PM, Cole Robinson wrote:
>>>> This requires drivers to opt in to handle the raw modelstr
>>>> network model, all others will error if a passed in XML value
>>>> is not in the model enum.
>>>>
>>>> Enable this feature for libxl/xen/xm and qemu drivers
>>>>
>>>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>>>> ---
>>>>   src/conf/domain_conf.c        | 9 +++++++++
>>>>   src/conf/domain_conf.h        | 1 +
>>>>   src/libxl/libxl_domain.c      | 1 +
>>>>   src/qemu/qemu_domain.c        | 3 ++-
>>>>   src/security/virt-aa-helper.c | 3 ++-
>>>>   5 files changed, 15 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>>> index f58a6d77b9..f06070d740 100644
>>>> --- a/src/conf/domain_conf.c
>>>> +++ b/src/conf/domain_conf.c
>>>> @@ -5265,6 +5265,15 @@
>>>> virDomainDeviceDefPostParseCheckFeatures(virDomainDeviceDefPtr dev,
>>>>           virDomainDeviceDefCheckUnsupportedMemoryDevice(dev) < 0)
>>>>           return -1;
>>>>   +    if (UNSUPPORTED(VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING) &&
>>>> +        dev->type == VIR_DOMAIN_DEVICE_NET &&
>>>> +        dev->data.net->modelstr) {
>>>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>>> +                       _("driver does not support net model '%s'"),
>>>> +                       dev->data.net->modelstr);
>>>> +        return -1;
>>>> +    }
>>>> +
>>>>       return 0;
>>>>   }
>>>>   #undef UNSUPPORTED
>>>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>>>> index 505982ec0e..14b831ffac 100644
>>>> --- a/src/conf/domain_conf.h
>>>> +++ b/src/conf/domain_conf.h
>>>> @@ -2769,6 +2769,7 @@ typedef enum {
>>>>       VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5),
>>>>       VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6),
>>>>       VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT = (1 << 7),
>>>> +    VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING = (1 << 8),
>>>>   } virDomainDefFeatures;
>>>>     diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>>>> index ffafa7967d..dc974dc926 100644
>>>> --- a/src/libxl/libxl_domain.c
>>>> +++ b/src/libxl/libxl_domain.c
>>>> @@ -424,6 +424,7 @@ virDomainDefParserConfig
>>>> libxlDomainDefParserConfig = {
>>>>       .macPrefix = { 0x00, 0x16, 0x3e },
>>>>       .devicesPostParseCallback = libxlDomainDeviceDefPostParse,
>>>>       .domainPostParseCallback = libxlDomainDefPostParse,
>>>> +    .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING,
>>>>   };
>>>>     diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>>> index 6718dfbc22..1214e32be4 100644
>>>> --- a/src/qemu/qemu_domain.c
>>>> +++ b/src/qemu/qemu_domain.c
>>>> @@ -6967,7 +6967,8 @@ virDomainDefParserConfig
>>>> virQEMUDriverDomainDefParserConfig = {
>>>>                   VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN |
>>>>                   VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS |
>>>>                   VIR_DOMAIN_DEF_FEATURE_USER_ALIAS |
>>>> -                VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT,
>>>> +                VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
>>>> +                VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING,
>>>>   };
>>>>     diff --git a/src/security/virt-aa-helper.c
>>>> b/src/security/virt-aa-helper.c
>>>> index 989dcf1784..c250c61cb6 100644
>>>> --- a/src/security/virt-aa-helper.c
>>>> +++ b/src/security/virt-aa-helper.c
>>>> @@ -645,7 +645,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr)
>>>>   virDomainDefParserConfig virAAHelperDomainDefParserConfig = {
>>>>       .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG |
>>>>                   VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN |
>>>> -                VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS,
>>>> +                VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS |
>>>> +                VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING,
>>>>   };
>>>>     static int
>>>>
>>>
>>> You've changed some other drivers too. Should we enable this feature for
>>> them too?
>>>
>>
>> The other drivers are all changed to only handle known enum values. This
>> feature is only enabled for drivers that still contain code which may
>> process the raw modelstr. So that's libxl and qemu (and by extension
>> virt-aa-helper which processes XML). So I think this piece is correct.
> 
> I see this has broken the virt-manager test suite which passes various
> invented model names to the test driver. Given that this is the test
> driver, this isn't too serious, but probably should be called out in
> the release notes if you've not already done so.
> 

I fixed the virt-manager test suite to not pass in unknown models so CI
should be fixed

This XML feature should be enabled for the test driver though, at least
until we get 'full' enum coverage in xen and qemu drivers (for example
right now test driver with model=pcnet will be rejected because it's not
yet in the enum, even though it definitely should be). I'll send a patch
for that.

I think release notes should only be added after the next round of
changes which will filling in more enum content, and maybe taint VMs if
they are using model passthrough (not sure yet, patches aren't written yet).

Thanks,
Cole