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

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(a)redhat.com&gt; >>> --- >>>   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.