Re: [libvirt] [PATCH 1/4] xen: Make xenDaemon*DeviceFlags errors less confusing
> When a user calls to
virDomain{Attach,Detach,Update}DeviceFlags() with
> flags == VIR_DOMAIN_DEVICE_MODIFY_LIVE on an inactive guest running on
> an old Xen hypervisor (such as RHEL-5) xend_internal driver reports:
>
> Xend version does not support modifying persistent config
>
> which is pretty confusing since no-one requested to modify persistent
> config.
Hmm - given the recent discussion on vcpus (which is probably what made
you look at this, right?)...
No :-) I was just fixing stuff at this area and noticed this confusing error
message.
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index fce0233..1318bd4 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -3878,6 +3878,12 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char
*xml,
> priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>
> if (domain->id< 0) {
> + /* Cannot modify live config if domain is inactive */
> + if (flags& VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> + virXendError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("Cannot modify live config if domain is
inactive"));
> + return -1;
> + }
Should we always error out if _LIVE and inactive, or should we
special-case _CONFIG|_LIVE by silently ignoring the _LIVE flag on
inactive domains?
Since there is the _CURRENT variant of the flag, erroring out if _LIVE and
inactive seems like the right thing to do for me. One can always use _CONFIG |
_CURRENT to change everything regardless on current state of the guest.
Anyway, my patch is not aimed at solving the "to error out or not to error
out" question but rather swapping two checks to get errors which are more
logic. I think I should have used more context for the patch to make this
clearer. Sorry about that.
Jirka