[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] virDomainTaintFlags: Introduce VIR_DOMAIN_TAINT_HOOK



On 02/04/2014 05:49 PM, Michal Privoznik wrote:
> This new flag is to be used for tainting domains which
> XML definition was altered at runtime by a hook script.
>
> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  src/conf/domain_conf.c | 3 ++-
>  src/conf/domain_conf.h | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 28e24f9..98ac8c8 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -107,7 +107,8 @@ VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
>                "shell-scripts",
>                "disk-probing",
>                "external-launch",
> -              "host-cpu");
> +              "host-cpu",
> +              "hook-script");

So I came back to this series after considering network tainting again.
In the case of networks, your patch just always tainted the network
whenever a hook script was present. But in the case of domains, you're
only tainting it if the hook script modified the XML *and* libvirt
accepted/used that modified XML.

This makes me think two things:

1) we should probably be consistent, so if we only taint the domain if
the hook modifies the XML and we use that XML, then maybe we shouldn't
taint networks just because a hook script was called (or maybe domains
should always get a "hook-script" taint if a script is run at all, and a
different taint if the hook modifies the XML - see (2))

2) The real reason we're tainting the domain here is because a hook
modified the xml, NOT just because a hook was run, so the reason should
probably be something like "hook-modified-xml". In the future, we may
want to also taint all domains that had a script run at all, and in that
case we would still have "hook-script" available to use.


Other than that, this and PATCH 2/2 are fine - ACK.
>  
>  VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST,
>                "qemu",
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index d8f2e49..dc5f8a1 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2109,6 +2109,7 @@ enum virDomainTaintFlags {
>      VIR_DOMAIN_TAINT_DISK_PROBING,     /* Relying on potentially unsafe disk format probing */
>      VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH,  /* Externally launched guest domain */
>      VIR_DOMAIN_TAINT_HOST_CPU,         /* Host CPU passthrough in use */
> +    VIR_DOMAIN_TAINT_HOOK,             /* Domain (possibly) changed via hook script */
>  
>      VIR_DOMAIN_TAINT_LAST
>  };


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]