[pve-devel] [PATCH qemu-server 5/5] hotplug_pending: allow partial fast plugging

[Stefan Reiter]

On 1/28/20 4:03 PM, Oguz Bektas wrote:
> adds a loop after the main fastplug loop, to check if any of the options
> are partially fast pluggable.
> 
> these are defined in $partial_fast_plug_option
> 
> our first use case is the fstrim_cloned_disks option of the
> qemu-guest-agent
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>   PVE/QemuConfig.pm |  7 +++++++
>   PVE/QemuServer.pm | 20 ++++++++++++++++++++
>   2 files changed, 27 insertions(+)
> 
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index 1ba728a..b9fdfbb 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -399,6 +399,13 @@ sub __snapshot_foreach_volume {
>   
>       PVE::QemuServer::foreach_drive($conf, $func);
>   }
> +
> +sub get_partial_fast_plug_map {
> +    my ($class) = @_;
> +
> +    return $PVE::QemuServer::partial_fast_plug_option;
> +}
> +
>   # END implemented abstract methods from PVE::AbstractConfig
>   
>   1;
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 6a8dc16..72d81ff 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4691,6 +4691,20 @@ my $fast_plug_option = {
>       'tags' => 1,
>   };
>   
> +# name
> +# -> fmt -> format variable
> +# -> properties -> fastpluggable options hash
> +our $partial_fast_plug_option = {
> +    agent => {
> +	fmt => $agent_fmt,
> +	properties => {
> +	    fstrim_cloned_disks => 1
> +	},
> +    },
> +};

I assume you called it _option because of the $fast_plug_option above, 
but then you call it _map in QemuConfig above. I think either option 
sounds reasonable, but I'd settle on one.

> +
> +
> +

useless whitespace

>   # hotplug changes in [PENDING]
>   # $selection hash can be used to only apply specified options, for
>   # example: { cores => 1 } (only apply changed 'cores')
> @@ -4720,6 +4734,12 @@ sub vmconfig_hotplug_pending {
>   	}
>       }
>   
> +    foreach my $opt (keys %{$conf->{pending}}) {
> +	if ($partial_fast_plug_option->{$opt}) {
> +	    PVE::QemuConfig->partial_fast_plug($conf, $opt);
> +	}
> +    }
> +

The non-partial fast-plugging code above this one conditionally sets 
$changes to indicate the need for the config to be written out before 
actual hotplugging commences (I assume so in case hotplugging fails, at 
least the fast-pluggable ones are persisted?).

This variable should probably be set by partial fast-plugging as well, 
and for that partial_fast_plug(...) needs to return whether it changed 
something. The $changes_left that is currently returned is useless here 
anyway, so it should be easy to replace and then set the $changes var 
correctly.

>       if ($changes) {
>   	PVE::QemuConfig->write_config($vmid, $conf);
>       }
>