[PATCH] libxl: Turn on user aliases

Michal Privoznik posted 1 patch 2 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/fccc7eaae14b6ebc9d5f2f3c1468db8302fe2521.1645098989.git.mprivozn@redhat.com
src/libxl/libxl_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] libxl: Turn on user aliases
Posted by Michal Privoznik 2 years, 2 months ago
When I implemented user aliases I've invented this
virDomainDefFeatures flag so that individual drivers can signal
support for user provided aliases. The reasoning was that a
device alias might be part of guest ABI, or used in a different
way then in QEMU. Well, neither applies to the libxl driver, so
it's safe to allow user aliases there.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/231
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libxl/libxl_domain.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index c91e531a9a..0816c5baa4 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -452,7 +452,8 @@ virDomainDefParserConfig libxlDomainDefParserConfig = {
     .domainPostParseCallback = libxlDomainDefPostParse,
     .domainValidateCallback = libxlDomainDefValidate,
 
-    .features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
+    .features = VIR_DOMAIN_DEF_FEATURE_USER_ALIAS |
+                VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
                 VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING,
 };
 
-- 
2.34.1

Re: [PATCH] libxl: Turn on user aliases
Posted by Jim Fehlig 2 years, 1 month ago
On 2/17/22 04:56, Michal Privoznik wrote:
> When I implemented user aliases I've invented this
> virDomainDefFeatures flag so that individual drivers can signal
> support for user provided aliases. The reasoning was that a
> device alias might be part of guest ABI, or used in a different
> way then in QEMU. Well, neither applies to the libxl driver, so
> it's safe to allow user aliases there.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/231
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   src/libxl/libxl_domain.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

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

Regards,
Jim

> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index c91e531a9a..0816c5baa4 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -452,7 +452,8 @@ virDomainDefParserConfig libxlDomainDefParserConfig = {
>       .domainPostParseCallback = libxlDomainDefPostParse,
>       .domainValidateCallback = libxlDomainDefValidate,
>   
> -    .features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
> +    .features = VIR_DOMAIN_DEF_FEATURE_USER_ALIAS |
> +                VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
>                   VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING,
>   };
>
Re: [PATCH] libxl: Turn on user aliases
Posted by Jim Fehlig 2 years, 1 month ago
On 2/17/22 04:56, Michal Privoznik wrote:
> When I implemented user aliases I've invented this
> virDomainDefFeatures flag so that individual drivers can signal
> support for user provided aliases. The reasoning was that a
> device alias might be part of guest ABI, or used in a different
> way then in QEMU. Well, neither applies to the libxl driver, so
> it's safe to allow user aliases there.

I suppose it is safe, but does it make sense since aliases are not used by the 
driver in any way, and not supported by libxl?

Regards,
Jim

> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/231
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   src/libxl/libxl_domain.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index c91e531a9a..0816c5baa4 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -452,7 +452,8 @@ virDomainDefParserConfig libxlDomainDefParserConfig = {
>       .domainPostParseCallback = libxlDomainDefPostParse,
>       .domainValidateCallback = libxlDomainDefValidate,
>   
> -    .features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
> +    .features = VIR_DOMAIN_DEF_FEATURE_USER_ALIAS |
> +                VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
>                   VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING,
>   };
>
Re: [PATCH] libxl: Turn on user aliases
Posted by Peter Krempa 2 years, 1 month ago
On Tue, Mar 01, 2022 at 09:40:30 -0700, Jim Fehlig wrote:
> On 2/17/22 04:56, Michal Privoznik wrote:
> > When I implemented user aliases I've invented this
> > virDomainDefFeatures flag so that individual drivers can signal
> > support for user provided aliases. The reasoning was that a
> > device alias might be part of guest ABI, or used in a different
> > way then in QEMU. Well, neither applies to the libxl driver, so
> > it's safe to allow user aliases there.
> 
> I suppose it is safe, but does it make sense since aliases are not used by
> the driver in any way, and not supported by libxl?

In that case at least the libvirt side user-visible aspect of it does
make sense. Users can pick a stable alias for their device if they care
about that for any reason.

In the qemu impl we decided to propagate it to the hypervisor, but that
should be considered an implementation detail as the IDs of objects
(mostly) don't have guest visible meaning.
Re: [PATCH] libxl: Turn on user aliases
Posted by Jim Fehlig 2 years, 1 month ago
On 3/2/22 01:29, Peter Krempa wrote:
> On Tue, Mar 01, 2022 at 09:40:30 -0700, Jim Fehlig wrote:
>> On 2/17/22 04:56, Michal Privoznik wrote:
>>> When I implemented user aliases I've invented this
>>> virDomainDefFeatures flag so that individual drivers can signal
>>> support for user provided aliases. The reasoning was that a
>>> device alias might be part of guest ABI, or used in a different
>>> way then in QEMU. Well, neither applies to the libxl driver, so
>>> it's safe to allow user aliases there.
>>
>> I suppose it is safe, but does it make sense since aliases are not used by
>> the driver in any way, and not supported by libxl?
> 
> In that case at least the libvirt side user-visible aspect of it does
> make sense. Users can pick a stable alias for their device if they care
> about that for any reason.

Nod, agree it does make sense :-).

Regards,
Jim
Re: [PATCH] libxl: Turn on user aliases
Posted by Laine Stump 2 years, 1 month ago
On 3/2/22 3:29 AM, Peter Krempa wrote:
> On Tue, Mar 01, 2022 at 09:40:30 -0700, Jim Fehlig wrote:
>> On 2/17/22 04:56, Michal Privoznik wrote:
>>> When I implemented user aliases I've invented this
>>> virDomainDefFeatures flag so that individual drivers can signal
>>> support for user provided aliases. The reasoning was that a
>>> device alias might be part of guest ABI, or used in a different
>>> way then in QEMU. Well, neither applies to the libxl driver, so
>>> it's safe to allow user aliases there.
>>
>> I suppose it is safe, but does it make sense since aliases are not used by
>> the driver in any way, and not supported by libxl?
> 
> In that case at least the libvirt side user-visible aspect of it does
> make sense. Users can pick a stable alias for their device if they care
> about that for any reason.

...for example, in the qemu driver, the alias can be used as an index to 
match when unplugging or updating a device; in the case of an 
<interface>, in the past you could match the device to unplug/update by 
looking at the MAC address or PCI address, but the MAC address isn't 
necessarily unique, and PCI address is usually determined by libvirt, so 
the management application may not know it. But the management 
application can just explicitly provide an alias in the <interface> XML 
when the domain is defined, and then later use that as the key when 
unplugging/updating.

Matching on alias when searching for an <interface> was added for use by 
the QEMU driver back in commit 114e3b4232, but fortunately this search 
is done by virDomainNetFindIdx(), which is also used by the libxl 
interface update and unplug functions, so libxl will get this 
functionality for free once Michal's patch is pushed.

(I haven't looked at whether or not other types of devices are using 
alias in this manner either in the qemu or libxl drivers, but I assume 
at least some of them are)

> 
> In the qemu impl we decided to propagate it to the hypervisor, but that
> should be considered an implementation detail as the IDs of objects
> (mostly) don't have guest visible meaning.
>