[PATCH v2 3/3] xenconfig: add support for 'seize' option of a PCI device

[Jim Fehlig]

On 4/24/20 9:07 PM, Marek Marczykowski-Górecki wrote:
> Add support for xl.cfg(5) pci device 'seize' option in
> domXML-to-xenconfig converter. And a test for it.
> It is functional equivalent of 'managed' attribute of a hostdev, so map
> it directly.

This is fine for the converter. We'd never want to set the seize field of 
libxl_device_pci struct since that would interfere with libvirt's management of 
the device.

> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> ---
> Changes in v2:
>   - new patch
> ---
>   src/libxl/xen_common.c                   | 19 +++++++++++++------
>   tests/xlconfigdata/test-fullvirt-pci.cfg |  2 +-
>   tests/xlconfigdata/test-fullvirt-pci.xml |  2 +-
>   3 files changed, 15 insertions(+), 8 deletions(-)

Reviewed-by: Jim Fehlig <jfehlig at suse.com>

This one could really be pushed separately but it depends on the test files you 
added in the previous patch. That's fine though. I'll wait and push it with the 
others after we've vetted the permissive attribute.

Regards,
Jim

> 
> diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
> index 050b2a0..8ae4aaa 100644
> --- a/src/libxl/xen_common.c
> +++ b/src/libxl/xen_common.c
> @@ -382,6 +382,7 @@ xenParsePCI(char *entry)
>       int slotID;
>       int funcID;
>       virTristateBool permissivebool = VIR_TRISTATE_BOOL_ABSENT;
> +    virTristateBool seizebool = VIR_TRISTATE_BOOL_ABSENT;
>   
>       domain[0] = bus[0] = slot[0] = func[0] = '\0';
>   
> @@ -432,18 +433,23 @@ xenParsePCI(char *entry)
>           data++;
>           if (!(nextkey = strchrnul(key, ',')))
>               return NULL;
> -        if (STRPREFIX(key, "permissive=")) {
> +        if (STRPREFIX(key, "permissive=") || STRPREFIX(key, "seize=")) {
>               char valuestr[5];
>               int valueint;
> +            virTristateBool valuebool;
>               if (virStrncpy(valuestr, data, (nextkey - data), sizeof(valuestr)) < 0) {
>                   virReportError(VIR_ERR_INTERNAL_ERROR,
> -                        _("Permissive %s too big for destination"), data);
> +                        _("%s %s too big for destination"), key, data);
>                   return NULL;
>               }
>               /* xl.cfg(5) specifies false as 0 and true as any other numeric value */
>               if (virStrToLong_i(valuestr, NULL, 10, &valueint) < 0)
>                   return NULL;
> -            permissivebool = valueint ? VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
> +            valuebool = valueint ? VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
> +            if (STRPREFIX(key, "permissive="))
> +                permissivebool = valuebool;
> +            else if (STRPREFIX(key, "seize="))
> +                seizebool = valuebool;
>           }
>       }
>   
> @@ -459,7 +465,7 @@ xenParsePCI(char *entry)
>       if (!(hostdev = virDomainHostdevDefNew()))
>          return NULL;
>   
> -    hostdev->managed = false;
> +    hostdev->managed = seizebool == VIR_TRISTATE_BOOL_YES;
>       hostdev->permissive = permissivebool;
>       hostdev->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
>       hostdev->source.subsys.u.pci.addr.domain = domainID;
> @@ -1898,12 +1904,13 @@ xenFormatPCI(virConfPtr conf, virDomainDefPtr def)
>                       break;
>               }
>   
> -            buf = g_strdup_printf("%04x:%02x:%02x.%x%s",
> +            buf = g_strdup_printf("%04x:%02x:%02x.%x%s%s",
>                                     def->hostdevs[i]->source.subsys.u.pci.addr.domain,
>                                     def->hostdevs[i]->source.subsys.u.pci.addr.bus,
>                                     def->hostdevs[i]->source.subsys.u.pci.addr.slot,
>                                     def->hostdevs[i]->source.subsys.u.pci.addr.function,
> -                                  permissive_str);
> +                                  permissive_str,
> +                                  def->hostdevs[i]->managed ? ",seize=1" : "");
>   
>               if (VIR_ALLOC(val) < 0) {
>                   VIR_FREE(buf);
> diff --git a/tests/xlconfigdata/test-fullvirt-pci.cfg b/tests/xlconfigdata/test-fullvirt-pci.cfg
> index 5a3f572..dcf2acd 100644
> --- a/tests/xlconfigdata/test-fullvirt-pci.cfg
> +++ b/tests/xlconfigdata/test-fullvirt-pci.cfg
> @@ -17,7 +17,7 @@ sdl = 0
>   vnc = 1
>   vncunused = 1
>   vnclisten = "127.0.0.1"
> -pci = [ "0000:01:1a.1", "0000:02:00.0,permissive=1" ]
> +pci = [ "0000:01:1a.1", "0000:02:00.0,permissive=1,seize=1" ]
>   parallel = "none"
>   serial = "none"
>   builder = "hvm"
> diff --git a/tests/xlconfigdata/test-fullvirt-pci.xml b/tests/xlconfigdata/test-fullvirt-pci.xml
> index dec390a..30fa1f5 100644
> --- a/tests/xlconfigdata/test-fullvirt-pci.xml
> +++ b/tests/xlconfigdata/test-fullvirt-pci.xml
> @@ -42,7 +42,7 @@
>           <address domain='0x0000' bus='0x01' slot='0x1a' function='0x1'/>
>         </source>
>       </hostdev>
> -    <hostdev mode='subsystem' type='pci' managed='no' permissive='yes'>
> +    <hostdev mode='subsystem' type='pci' managed='yes' permissive='yes'>
>         <driver name='xen'/>
>         <source>
>           <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
>