[libvirt] [PATCH] conf: Fix ABI stability check for spicevmc channel
Change device type of a virtio channel from/to spicevmc is not a user visible change. However, spicevmc channels use different default target name than other virtio channels. To maintain ABI stability during this change target name must be explicitly specified (and equal) in both configurations. --- src/conf/domain_conf.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2e73c6..a330b0e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10075,6 +10075,15 @@ static bool virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, NULLSTR(dst->target.name), NULLSTR(src->target.name)); goto cleanup; } + if (src->source.type != dst->source.type && + (src->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC || + dst->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) && + !src->target.name) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Changing device type to/from spicevmc would" + " change default target channel name")); + goto cleanup; + } break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: if (memcmp(src->target.addr, dst->target.addr, -- 1.7.11.1
On 08/17/2012 06:20 AM, Jiri Denemark wrote:
Change device type of a virtio channel from/to spicevmc is not a user visible change. However, spicevmc channels use different default target name than other virtio channels. To maintain ABI stability during this change target name must be explicitly specified (and equal) in both configurations.
Can we go from 'non-spice, no name' to 'spice, explicit name that matches old non-spice default name'? Can we go from 'spice, no name' to 'non-spice, explicit name that matches old spice default name'? Your patch prevents those, even if it is technically possible. For that matter, can we even go from 'non-spice, no name' to 'non-spice, explicit default name'? If we can, then the user can achieve the earlier task through two steps.
--- src/conf/domain_conf.c | 9 +++++++++ 1 file changed, 9 insertions(+)
We may have more work given the above question, but I'd rather be stricter than necessary and safe than allow a real problem, so I'm okay with this patch as-is, even if it is too tight for some corner cases of adding a compatible name. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Aug 17, 2012 at 06:51:22 -0600, Eric Blake wrote:
On 08/17/2012 06:20 AM, Jiri Denemark wrote:
Change device type of a virtio channel from/to spicevmc is not a user visible change. However, spicevmc channels use different default target name than other virtio channels. To maintain ABI stability during this change target name must be explicitly specified (and equal) in both configurations.
Can we go from 'non-spice, no name' to 'spice, explicit name that matches old non-spice default name'? Can we go from 'spice, no name' to 'non-spice, explicit name that matches old spice default name'? Your patch prevents those, even if it is technically possible.
For that matter, can we even go from 'non-spice, no name' to 'non-spice, explicit default name'? If we can, then the user can achieve the earlier task through two steps.
In theory, both can be done but both cases are forbidden by ABI stability checks. The main reason is that ABI stability check is a generic code which has no idea what the default values are. Moving the defaults from qemu driver to conf is certainly doable but I think we should avoid doing so unless we absolutely have to. Jirka
On Fri, Aug 17, 2012 at 06:51:22 -0600, Eric Blake wrote:
On 08/17/2012 06:20 AM, Jiri Denemark wrote:
Change device type of a virtio channel from/to spicevmc is not a user visible change. However, spicevmc channels use different default target name than other virtio channels. To maintain ABI stability during this change target name must be explicitly specified (and equal) in both configurations. ... We may have more work given the above question, but I'd rather be stricter than necessary and safe than allow a real problem, so I'm okay with this patch as-is, even if it is too tight for some corner cases of adding a compatible name.
ACK.
Pushed, thanks. Jirka
participants (2)
-
Eric Blake -
Jiri Denemark