[libvirt] [PATCH 0/7] Add APIs for individual vCPU state modification using the guest agent

As a first step, add the guest-agent part. The real hotplug stuff will follow shortly. Please note that the setter API will be used as template for the real stuff too. Peter Krempa (7): rpcgen: Add support for generating funcs returning alloc'd typed params lib: Add API to query guest vcpu info using guest agent lib: Add API to set individual vcpu usage in the guest via guest agent virsh: Add command 'guestvcpus' implementing virDomain(GS)etGuestVcpus qemu: agent: Make setting of vcpus more robust qemu: Implement virDomainGetGuestVcpus qemu: Implement virDomainSetGuestVcpus include/libvirt/libvirt-domain.h | 10 ++ src/driver-hypervisor.h | 14 +++ src/libvirt-domain.c | 105 ++++++++++++++++++++ src/libvirt_public.syms | 2 + src/qemu/qemu_agent.c | 83 +++++++++++++--- src/qemu/qemu_agent.h | 2 + src/qemu/qemu_driver.c | 210 ++++++++++++++++++++++++++++++++++++--- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 39 +++++++- src/remote_protocol-structs | 18 ++++ src/rpc/gendispatch.pl | 45 ++++++++- tests/qemuagenttest.c | 44 ++++---- tools/virsh-domain.c | 93 +++++++++++++++++ tools/virsh.pod | 10 ++ 14 files changed, 625 insertions(+), 52 deletions(-) -- 2.8.3

Since it's rather tedious to write the dispatchers for functions that return an array of typed parameters (which are rather common) let's add some rpcgen code to generate them. --- src/remote/remote_protocol.x | 5 +++++ src/rpc/gendispatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 0170c0c..cec6bd2 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -414,6 +414,11 @@ struct remote_domain_disk_error { * insert@<offset> comment to indicate the offset in the parameter list of * the function to be called. * + * For cases where the API allocates memory and fills the arguments (mostly + * typed parameters) a similar comment indicates the type and offset + * of the variable to be filled with the count of returned elements. + * alloc@<offset>@unsigned int@<count offset> + * * Dynamic opaque and remote_nonnull_string arrays can be annotated with an * optional typecast */ diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 5564b2e..173189c 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -862,6 +862,25 @@ elsif ($mode eq "server") { $single_ret_var = $2; $single_ret_by_ref = 0; $single_ret_check = " == NULL"; + } elsif ($ret_member =~ m/^remote_typed_param (\S+)<(\S+)>;\s*\/\*\s*alloc@(\d+)@([^@]+)@(\d+)\s*\*\//) { + push(@vars_list, "virTypedParameterPtr $1 = NULL"); + push(@vars_list, "$4 $1_len = 0"); + + $single_ret_by_ref = 1; + $single_ret_var = undef; + + splice(@args_list, int($3), 0, "&$1"); + splice(@args_list, int($5), 0, "&$1_len"); + + push(@ret_list, "if (virTypedParamsSerialize($1, $1_len,\n" . + " (virTypedParameterRemotePtr *) &ret->$1.$1_val,\n" . + " &ret->$1.$1_len,\n" . + " VIR_TYPED_PARAM_STRING_OKAY) < 0)\n" . + " goto cleanup;\n"); + + push(@free_list, " virTypedParamsFree($1, $1_len);"); + push(@free_list_on_error, "virTypedParamsRemoteFree((virTypedParameterRemotePtr) ret->params.params_val,\n" . + " ret->params.params_len);\n"); } elsif ($ret_member =~ m/^(\/)?\*/) { # ignore comments } else { @@ -1422,6 +1441,7 @@ elsif ($mode eq "client") { my $modern_ret_as_list = 0; my $modern_ret_struct_name = "undefined"; my $modern_ret_var_type = "undefined"; + my @custom_error_cleanup = (); if ($rettype ne "void" and scalar(@{$call->{ret_members}}) > 1) { @@ -1519,6 +1539,23 @@ elsif ($mode eq "client") { $single_ret_var = "vir${type_name}Ptr rv = NULL"; $single_ret_type = "vir${type_name}Ptr"; } + } elsif ($ret_member =~ m/^remote_typed_param (\S+)<(\S+)>;\s*\/\*\s*alloc@(\d+)@([^@]+)@(\d+)\s*\*\//) { + # handle self allocating arrays of typed parameters + splice(@args_list, int($3), 0, ("virTypedParameterPtr *$1")); + splice(@args_list, int($5), 0, ("$4 *n$1")); + push(@vars_list, "virTypedParameterPtr ret_params = NULL"); + push(@vars_list, "int ret_nparams = 0"); + # virTypedParamsDeserialize allocates the array if @params is null + push(@ret_list2, "if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.$1.$1_val,\n" . + " ret.$1.$1_len,\n" . + " $2,\n" . + " &ret_params,\n" . + " &ret_nparams) < 0)\n" . + " goto cleanup;\n"); + push(@ret_list2, "*$1 = ret_params;"); + push(@ret_list2, "*n$1 = ret_nparams;"); + push(@custom_error_cleanup, "virTypedParamsFree(ret_params, ret_nparams);\n"); + $single_ret_cleanup = 1; } elsif ($ret_member =~ m/^remote_typed_param (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) { splice(@args_list, int($3), 0, ("virTypedParameterPtr $1")); push(@ret_list2, "if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.$1.$1_val,\n" . @@ -1530,7 +1567,7 @@ elsif ($mode eq "client") { $single_ret_cleanup = 1; } elsif ($ret_member =~ m/^remote_typed_param (\S+)<\S+>;/) { # error out on unannotated arrays - die "remote_typed_param array without insert@<offset> annotation: $ret_member"; + die "remote_typed_param array without insert@... or alloc@... annotation: $ret_member"; } elsif ($ret_member =~ m/^int (\S+);/) { my $arg_name = $1; @@ -1876,6 +1913,12 @@ elsif ($mode eq "client") { if ($single_ret_as_list or $single_ret_cleanup or $modern_ret_as_list) { print "\n"; print "cleanup:\n"; + if (@custom_error_cleanup) { + print " if (rv != 0) {\n"; + print " "; + print join("\n ", @custom_error_cleanup); + print " }\n"; + } if ($modern_ret_as_list) { print " if (tmp_results) {\n"; print " for (i = 0; i < ret.$single_ret_list_name.${single_ret_list_name}_len; i++)\n"; -- 2.8.3

On Mon, Jun 20, 2016 at 04:34:15PM +0200, Peter Krempa wrote:
Since it's rather tedious to write the dispatchers for functions that return an array of typed parameters (which are rather common) let's add some rpcgen code to generate them. --- src/remote/remote_protocol.x | 5 +++++ src/rpc/gendispatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-)
ACK

Add a rather universal API implemented via typed params that will allow to query the guest agent for the state and possibly other aspects of guest vcpus. --- include/libvirt/libvirt-domain.h | 5 ++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 56 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 21 ++++++++++++++- src/remote_protocol-structs | 11 ++++++++ 7 files changed, 101 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index cba4fa5..5605a94 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4097,4 +4097,9 @@ int virDomainRename(virDomainPtr dom, const char *new_name, unsigned int flags); +int virDomainGetGuestVcpus(virDomainPtr domain, + virTypedParameterPtr *params, + unsigned int *nparams, + unsigned int flags); + #endif /* __VIR_LIBVIRT_DOMAIN_H__ */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 5ab5775..93af2b6 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1239,6 +1239,12 @@ typedef int (*virDrvConnectUnregisterCloseCallback)(virConnectPtr conn, virConnectCloseFunc cb); +typedef int +(*virDrvDomainGetGuestVcpus)(virDomainPtr domain, + virTypedParameterPtr *params, + unsigned int *nparams, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1475,6 +1481,7 @@ struct _virHypervisorDriver { virDrvConnectRegisterCloseCallback connectRegisterCloseCallback; virDrvConnectUnregisterCloseCallback connectUnregisterCloseCallback; virDrvDomainMigrateStartPostCopy domainMigrateStartPostCopy; + virDrvDomainGetGuestVcpus domainGetGuestVcpus; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 73ae369..0971a96 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11835,3 +11835,59 @@ virDomainInterfaceFree(virDomainInterfacePtr iface) VIR_FREE(iface); } + + +/** + * virDomainGetGuestVcpus: + * @domain: pointer to domain object + * @params: Pointer that will be filled with an array of typed parameters + * @nparams: pointer filled with number of elements in @params + * @flags: currently unused, callers shall pass 0 + * + * Queries the guest agent for state and information regarding vCPUs from + * guest's perspective. The reported data depends on the guest agent + * implementation. + * + * Reported fields stored in @params: + * 'vcpus': string containing bitmap representing vCPU ids as reported by the + * guest + * 'online': string containing bitmap representing online vCPUs as reported + * by the guest agent. + * 'offlinable': string containing bitmap representing ids of vCPUs that can be + * offlined + * + * This API requires the VM to run. The caller is responsible for calling + * virTypedParamsFree to free memory returned in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virDomainGetGuestVcpus(virDomainPtr domain, + virTypedParameterPtr *params, + unsigned int *nparams, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p, flags=%x", + params, nparams, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(params, error); + virCheckNonNullArgGoto(nparams, error); + + if (domain->conn->driver->domainGetGuestVcpus) { + int ret; + ret = domain->conn->driver->domainGetGuestVcpus(domain, params, nparams, + flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 06011ce..30801c5 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -736,6 +736,7 @@ LIBVIRT_2.0.0 { global: virConnectStoragePoolEventRegisterAny; virConnectStoragePoolEventDeregisterAny; + virDomainGetGuestVcpus; } LIBVIRT_1.3.3; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 84b6d58..6a580c6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7981,6 +7981,7 @@ static virHypervisorDriver hypervisor_driver = { .connectRegisterCloseCallback = remoteConnectRegisterCloseCallback, /* 1.3.2 */ .connectUnregisterCloseCallback = remoteConnectUnregisterCloseCallback, /* 1.3.2 */ .domainMigrateStartPostCopy = remoteDomainMigrateStartPostCopy, /* 1.3.3 */ + .domainGetGuestVcpus = remoteDomainGetGuestVcpus, /* 2.0.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index cec6bd2..f72918d 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -250,6 +250,9 @@ const REMOTE_DOMAIN_INTERFACE_MAX = 2048; /* Upper limit on number of IP addresses per interface */ const REMOTE_DOMAIN_IP_ADDR_MAX = 2048; +/* Upper limit on number of guest vcpu information entries */ +const REMOTE_DOMAIN_GUEST_VCPU_PARAMS_MAX = 64; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -3292,6 +3295,16 @@ struct remote_domain_event_callback_device_removal_failed_msg { remote_nonnull_string devAlias; }; +struct remote_domain_get_guest_vcpus_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_guest_vcpus_ret { + remote_typed_param params<REMOTE_DOMAIN_GUEST_VCPU_PARAMS_MAX>; /* alloc@1@unsigned int@2 */ +}; + + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5839,5 +5852,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370 + REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370, + + /** + * @generate: both + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_GET_GUEST_VCPUS = 371 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 3934e07..4e603db 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2751,6 +2751,16 @@ struct remote_domain_event_callback_device_removal_failed_msg { remote_nonnull_domain dom; remote_nonnull_string devAlias; }; +struct remote_domain_get_guest_vcpus_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_guest_vcpus_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3122,4 +3132,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY = 368, REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY = 369, REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370, + REMOTE_PROC_DOMAIN_GET_GUEST_VCPUS = 371, }; -- 2.8.3

On Mon, Jun 20, 2016 at 04:34:16PM +0200, Peter Krempa wrote:
Add a rather universal API implemented via typed params that will allow to query the guest agent for the state and possibly other aspects of guest vcpus. ---
[...]
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 73ae369..0971a96 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11835,3 +11835,59 @@ virDomainInterfaceFree(virDomainInterfacePtr iface)
VIR_FREE(iface); } + + +/** + * virDomainGetGuestVcpus: + * @domain: pointer to domain object + * @params: Pointer that will be filled with an array of typed parameters
s/Pointer/pointer/
+ * @nparams: pointer filled with number of elements in @params + * @flags: currently unused, callers shall pass 0 + * + * Queries the guest agent for state and information regarding vCPUs from + * guest's perspective. The reported data depends on the guest agent + * implementation. + * + * Reported fields stored in @params: + * 'vcpus': string containing bitmap representing vCPU ids as reported by the + * guest + * 'online': string containing bitmap representing online vCPUs as reported + * by the guest agent. + * 'offlinable': string containing bitmap representing ids of vCPUs that can be + * offlined
ACK

To allow finer-grained control of vcpu state using guest agent this API can be used to individually set the state of the vCPU. This will allow to better control NUMA enabled guests and/or test various vCPU configurations. --- include/libvirt/libvirt-domain.h | 5 ++++ src/driver-hypervisor.h | 7 ++++++ src/libvirt-domain.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 +++++++++++- src/remote_protocol-structs | 7 ++++++ 7 files changed, 84 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 5605a94..5b80b85 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4102,4 +4102,9 @@ int virDomainGetGuestVcpus(virDomainPtr domain, unsigned int *nparams, unsigned int flags); +int virDomainSetGuestVcpus(virDomainPtr domain, + const char *cpumap, + int state, + unsigned int flags); + #endif /* __VIR_LIBVIRT_DOMAIN_H__ */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 93af2b6..5cd1fdf 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1245,6 +1245,12 @@ typedef int unsigned int *nparams, unsigned int flags); +typedef int +(*virDrvDomainSetGuestVcpus)(virDomainPtr domain, + const char *cpumap, + int state, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1482,6 +1488,7 @@ struct _virHypervisorDriver { virDrvConnectUnregisterCloseCallback connectUnregisterCloseCallback; virDrvDomainMigrateStartPostCopy domainMigrateStartPostCopy; virDrvDomainGetGuestVcpus domainGetGuestVcpus; + virDrvDomainSetGuestVcpus domainSetGuestVcpus; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0971a96..11f20af 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11891,3 +11891,52 @@ virDomainGetGuestVcpus(virDomainPtr domain, virDispatchError(domain->conn); return -1; } + + +/** + * virDomainSetGuestVcpus: + * @domain: pointer to domain object + * @cpumap: text representation of a bitmap of vcpus to set + * @state: 0 to disable/1 to enable cpus described by @cpumap + * @flags: currently unused, callers shall pass 0 + * + * Sets state of individual vcpus described by @cpumap via guest agent. Other + * vcpus are not modified. + * + * This API requires the VM to run. Various hypervisors or guest agent + * implementation may limit to operate on just 1 vCPU per call. + * + * Note that OSes (notably Linux) may require vCPU 0 to stay online to support + * low-level features a S3 sleep. + * + * Returns 0 on success, -1 on error. + */ +int +virDomainSetGuestVcpus(virDomainPtr domain, + const char *cpumap, + int state, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(domain, "cpumap='%s' state=%x flags=%x", + NULLSTR(cpumap), state, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckNonNullArgGoto(cpumap, error); + + if (domain->conn->driver->domainSetGuestVcpus) { + int ret; + ret = domain->conn->driver->domainSetGuestVcpus(domain, cpumap, state, + flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 30801c5..b6d2dfd 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -737,6 +737,7 @@ LIBVIRT_2.0.0 { virConnectStoragePoolEventRegisterAny; virConnectStoragePoolEventDeregisterAny; virDomainGetGuestVcpus; + virDomainSetGuestVcpus; } LIBVIRT_1.3.3; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 6a580c6..3f9d812 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7982,6 +7982,7 @@ static virHypervisorDriver hypervisor_driver = { .connectUnregisterCloseCallback = remoteConnectUnregisterCloseCallback, /* 1.3.2 */ .domainMigrateStartPostCopy = remoteDomainMigrateStartPostCopy, /* 1.3.3 */ .domainGetGuestVcpus = remoteDomainGetGuestVcpus, /* 2.0.0 */ + .domainSetGuestVcpus = remoteDomainSetGuestVcpus, /* 2.0.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f72918d..d11bfdf 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3304,6 +3304,13 @@ struct remote_domain_get_guest_vcpus_ret { remote_typed_param params<REMOTE_DOMAIN_GUEST_VCPU_PARAMS_MAX>; /* alloc@1@unsigned int@2 */ }; +struct remote_domain_set_guest_vcpus_args { + remote_nonnull_domain dom; + remote_nonnull_string cpumap; + int state; + unsigned int flags; +}; + /*----- Protocol. -----*/ @@ -5858,5 +5865,11 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_GET_GUEST_VCPUS = 371 + REMOTE_PROC_DOMAIN_GET_GUEST_VCPUS = 371, + + /** + * @generate: both + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_SET_GUEST_VCPUS = 372 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4e603db..0d89b15 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2761,6 +2761,12 @@ struct remote_domain_get_guest_vcpus_ret { remote_typed_param * params_val; } params; }; +struct remote_domain_set_guest_vcpus_args { + remote_nonnull_domain dom; + remote_nonnull_string cpumap; + int state; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3133,4 +3139,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY = 369, REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370, REMOTE_PROC_DOMAIN_GET_GUEST_VCPUS = 371, + REMOTE_PROC_DOMAIN_SET_GUEST_VCPUS = 372, }; -- 2.8.3

On Mon, Jun 20, 2016 at 04:34:17PM +0200, Peter Krempa wrote:
To allow finer-grained control of vcpu state using guest agent this API can be used to individually set the state of the vCPU.
This will allow to better control NUMA enabled guests and/or test various vCPU configurations. ---
ACK

On Mon, Jun 20, 2016 at 04:34:17PM +0200, Peter Krempa wrote:
To allow finer-grained control of vcpu state using guest agent this API can be used to individually set the state of the vCPU.
This will allow to better control NUMA enabled guests and/or test various vCPU configurations. ---
[...]
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0971a96..11f20af 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11891,3 +11891,52 @@ virDomainGetGuestVcpus(virDomainPtr domain, virDispatchError(domain->conn); return -1; } + + +/** + * virDomainSetGuestVcpus: + * @domain: pointer to domain object + * @cpumap: text representation of a bitmap of vcpus to set + * @state: 0 to disable/1 to enable cpus described by @cpumap + * @flags: currently unused, callers shall pass 0 + * + * Sets state of individual vcpus described by @cpumap via guest agent. Other + * vcpus are not modified.
It would be worth documenting how the bitmap should looks like as we do in virsh. This is a new bitmap representation in our APIs. And also add a reference to this description in the virDomainGetGuestVcpus API.
+ * + * This API requires the VM to run. Various hypervisors or guest agent + * implementation may limit to operate on just 1 vCPU per call. + * + * Note that OSes (notably Linux) may require vCPU 0 to stay online to support + * low-level features a S3 sleep. + * + * Returns 0 on success, -1 on error. + */

Add a straightforward implementation for using the new APIs. --- tools/virsh-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 10 ++++++ 2 files changed, 103 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bd6db47..ff2305f 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6762,6 +6762,93 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) return ret; } + +/* + * "guestvcpus" command + */ +static const vshCmdInfo info_guestvcpus[] = { + {.name = "help", + .data = N_("query or modify state of vcpu in the guest (via agent)") + }, + {.name = "desc", + .data = N_("Use the guest agent to query or set cpu state from guest's " + "point of view") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_guestvcpus[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL, + {.name = "cpumap", + .type = VSH_OT_STRING, + .help = N_("number of virtual CPUs") + }, + {.name = "enable", + .type = VSH_OT_BOOL, + .help = N_("enable cpus specified by cpumap") + }, + {.name = "disable", + .type = VSH_OT_BOOL, + .help = N_("disable cpus specified by cpumap") + }, + {.name = NULL} +}; + +static bool +cmdGuestvcpus(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool enable = vshCommandOptBool(cmd, "enable"); + bool disable = vshCommandOptBool(cmd, "disable"); + virTypedParameterPtr params = NULL; + unsigned int nparams = 0; + const char *cpumap = NULL; + int state = 0; + size_t i; + bool ret = false; + + VSH_EXCLUSIVE_OPTIONS_VAR(enable, disable); + VSH_REQUIRE_OPTION("enable", "cpumap"); + VSH_REQUIRE_OPTION("disable", "cpumap"); + + if (vshCommandOptStringReq(ctl, cmd, "cpumap", &cpumap)) + return false; + + if (cpumap && !(enable | disable)) { + vshError(ctl, _("One of options --enable or --disable is required by " + "option --cpumap")); + return false; + } + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (enable) + state = 1; + + if (cpumap) { + if (virDomainSetGuestVcpus(dom, cpumap, state, 0) < 0) + goto cleanup; + } else { + if (virDomainGetGuestVcpus(dom, ¶ms, &nparams, 0) < 0) + goto cleanup; + + for (i = 0; i < nparams; i++) { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); + } + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + virDomainFree(dom); + return ret; +} + + /* * "iothreadinfo" command */ @@ -13602,5 +13689,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_vncdisplay, .flags = 0 }, + {.name = "guestvcpus", + .handler = cmdGuestvcpus, + .opts = opts_guestvcpus, + .info = info_guestvcpus, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 8e3ba69..6af1328 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2501,6 +2501,16 @@ Both I<--live> and I<--config> flags may be given if I<cpulist> is present, but I<--current> is exclusive. If no flag is specified, behavior is different depending on hypervisor. +=item B<guestvcpus> I<domain> [[I<--enable>] | [I<--disable>]] [I<cpumap>] + +Query or change state of vCPUs from guest's point of view using the guest agent. +When invoked without I<cpumap> the guest is queried for available guest vCPUs, +their state and possibility to be offlined. + +If I<cpumap> is provided then one of I<--enable> or I<--disable> must be +provided too. The desired operation is then executed on the domain. + +See B<vcpupin> for I<cpumap> (as I<cpulist>). =item B<vncdisplay> I<domain> -- 2.8.3

On Mon, Jun 20, 2016 at 04:34:18PM +0200, Peter Krempa wrote:
Add a straightforward implementation for using the new APIs. --- tools/virsh-domain.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 10 ++++++ 2 files changed, 103 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bd6db47..ff2305f 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6762,6 +6762,93 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) return ret; }
+ +/* + * "guestvcpus" command + */ +static const vshCmdInfo info_guestvcpus[] = { + {.name = "help", + .data = N_("query or modify state of vcpu in the guest (via agent)") + }, + {.name = "desc", + .data = N_("Use the guest agent to query or set cpu state from guest's " + "point of view") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_guestvcpus[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL, + {.name = "cpumap", + .type = VSH_OT_STRING, + .help = N_("number of virtual CPUs") + },
I would go with "cpulist" and also change the help string, this is not correct.
+ {.name = "enable", + .type = VSH_OT_BOOL, + .help = N_("enable cpus specified by cpumap") + }, + {.name = "disable", + .type = VSH_OT_BOOL, + .help = N_("disable cpus specified by cpumap") + }, + {.name = NULL} +};
ACK

Documentation for the "guest-set-vcpus" command describes a proper algorithm how to set vcpus. This patch makes the following changes: - state of cpus that has not changed is not updated - if the command was partially successful the command is re-tried with the rest of the arguments to get a proper error message - code is more robust against mailicious guest agent - fix testsuite to the new semantics --- src/qemu/qemu_agent.c | 83 ++++++++++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_agent.h | 2 ++ src/qemu/qemu_driver.c | 13 -------- tests/qemuagenttest.c | 44 ++++++++++++-------------- 4 files changed, 92 insertions(+), 50 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index cbc0995..5bd767a 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1523,16 +1523,13 @@ qemuAgentGetVCPUs(qemuAgentPtr mon, return ret; } -/** - * Set the VCPU state using guest agent. - * - * Returns -1 on error, ninfo in case everything was successful and less than - * ninfo on a partial failure. - */ -int -qemuAgentSetVCPUs(qemuAgentPtr mon, - qemuAgentCPUInfoPtr info, - size_t ninfo) + +/* returns the value provided by the guest agent or -1 on internal error */ +static int +qemuAgentSetVCPUsCommand(qemuAgentPtr mon, + qemuAgentCPUInfoPtr info, + size_t ninfo, + int *nmodified) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -1541,6 +1538,8 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, virJSONValuePtr cpu = NULL; size_t i; + *nmodified = 0; + /* create the key data array */ if (!(cpus = virJSONValueNewArray())) goto cleanup; @@ -1552,6 +1551,12 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, if (!(cpu = virJSONValueNewObject())) goto cleanup; + /* don't set state for cpus that were not touched */ + if (!in->modified) + continue; + + (*nmodified)++; + if (virJSONValueObjectAppendNumberInt(cpu, "logical-id", in->id) < 0) goto cleanup; @@ -1564,6 +1569,11 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, cpu = NULL; } + if (*nmodified == 0) { + ret = 0; + goto cleanup; + } + if (!(cmd = qemuAgentMakeCommand("guest-set-vcpus", "a:vcpus", cpus, NULL))) @@ -1575,9 +1585,17 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) goto cleanup; - if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + if (qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + /* All negative values are invalid. Return of 0 is bougs since we wouldn't + * call the guest agent so that 0 cpus would be set successfully. Reporting + * more successfuly set vcpus that we've asked for is invalid */ + if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0 || + ret <= 0 || ret > *nmodified) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed return value")); + _("guest agent returned malformed or invalid return value")); + ret = -1; } cleanup: @@ -1589,6 +1607,45 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, } +/** + * Set the VCPU state using guest agent. + * + * Attempts to set the guest agent state for all cpus or until a proper error is + * reported by the guest agent. This may require multiple calls. + * + * Returns -1 on error, 0 on success. + */ +int +qemuAgentSetVCPUs(qemuAgentPtr mon, + qemuAgentCPUInfoPtr info, + size_t ninfo) +{ + int rv; + int nmodified; + size_t i; + + do { + if ((rv = qemuAgentSetVCPUsCommand(mon, info, ninfo, &nmodified)) < 0) + return -1; + + /* all vcpus were set successfully */ + if (rv == nmodified) + return 0; + + /* un-mark vcpus that were already set */ + for (i = 0; i < ninfo && rv > 0; i++) { + if (!info[i].modified) + continue; + + info[i].modified = false; + rv--; + } + } while (1); + + return 0; +} + + /* modify the cpu info structure to set the correct amount of cpus */ int qemuAgentUpdateCPUInfo(unsigned int nvcpus, @@ -1647,12 +1704,14 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, /* unplug */ if (cpuinfo[i].offlinable && cpuinfo[i].online) { cpuinfo[i].online = false; + cpuinfo[i].modified = true; nonline--; } } else if (nvcpus > nonline) { /* plug */ if (!cpuinfo[i].online) { cpuinfo[i].online = true; + cpuinfo[i].modified = true; nonline++; } } else { diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index c092504..6dd9c70 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -95,6 +95,8 @@ struct _qemuAgentCPUInfo { unsigned int id; /* logical cpu ID */ bool online; /* true if the CPU is activated */ bool offlinable; /* true if the CPU can be offlined */ + + bool modified; /* set to true if the vcpu state needs to be changed */ }; int qemuAgentGetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr *info); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d065e45..098840f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4804,19 +4804,6 @@ qemuDomainSetVcpusAgent(virDomainObjPtr vm, ret = qemuAgentSetVCPUs(qemuDomainGetAgent(vm), cpuinfo, ncpuinfo); qemuDomainObjExitAgent(vm); - if (ret < 0) - goto cleanup; - - if (ret < ncpuinfo) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to set state of cpu %d via guest agent"), - cpuinfo[ret-1].id); - ret = -1; - goto cleanup; - } - - ret = 0; - cleanup: VIR_FREE(cpuinfo); diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 60dafd9..4aa2c49 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -517,17 +517,15 @@ static const char testQemuAgentCPUResponse[] = "}"; static const char testQemuAgentCPUArguments1[] = - "[{\"logical-id\":0,\"online\":true}," - "{\"logical-id\":1,\"online\":false}," - "{\"logical-id\":2,\"online\":true}," - "{\"logical-id\":3,\"online\":false}]"; + "[{\"logical-id\":1,\"online\":false}]"; static const char testQemuAgentCPUArguments2[] = - "[{\"logical-id\":0,\"online\":true}," - "{\"logical-id\":1,\"online\":true}," - "{\"logical-id\":2,\"online\":true}," + "[{\"logical-id\":1,\"online\":true}," "{\"logical-id\":3,\"online\":true}]"; +static const char testQemuAgentCPUArguments3[] = + "[{\"logical-id\":3,\"online\":true}]"; + static int testQemuAgentCPU(const void *data) { @@ -566,43 +564,39 @@ testQemuAgentCPU(const void *data) goto cleanup; if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", - "{ \"return\" : 4 }", + "{ \"return\" : 1 }", "vcpus", testQemuAgentCPUArguments1, NULL) < 0) goto cleanup; - if ((nvcpus = qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), - cpuinfo, nvcpus)) < 0) - goto cleanup; - - if (nvcpus != 4) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "Expected '4' cpus updated , got '%d'", nvcpus); + if (qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), cpuinfo, nvcpus) < 0) goto cleanup; - } - /* try to hotplug two */ + /* try to hotplug two, second one will fail*/ if (qemuMonitorTestAddAgentSyncResponse(test) < 0) goto cleanup; if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", - "{ \"return\" : 4 }", + "{ \"return\" : 1 }", "vcpus", testQemuAgentCPUArguments2, NULL) < 0) goto cleanup; - if (qemuAgentUpdateCPUInfo(4, cpuinfo, nvcpus) < 0) + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) goto cleanup; - if ((nvcpus = qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), - cpuinfo, nvcpus)) < 0) + if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", + "{ \"error\" : \"random error\" }", + "vcpus", testQemuAgentCPUArguments3, + NULL) < 0) goto cleanup; - if (nvcpus != 4) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "Expected '4' cpus updated , got '%d'", nvcpus); + if (qemuAgentUpdateCPUInfo(4, cpuinfo, nvcpus) < 0) + goto cleanup; + + /* this should fail */ + if (qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), cpuinfo, nvcpus) != -1) goto cleanup; - } ret = 0; -- 2.8.3

On Mon, Jun 20, 2016 at 04:34:19PM +0200, Peter Krempa wrote:
Documentation for the "guest-set-vcpus" command describes a proper algorithm how to set vcpus. This patch makes the following changes:
- state of cpus that has not changed is not updated - if the command was partially successful the command is re-tried with the rest of the arguments to get a proper error message - code is more robust against mailicious guest agent
s/mailicious/malicious/
- fix testsuite to the new semantics --- src/qemu/qemu_agent.c | 83 ++++++++++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_agent.h | 2 ++ src/qemu/qemu_driver.c | 13 -------- tests/qemuagenttest.c | 44 ++++++++++++-------------- 4 files changed, 92 insertions(+), 50 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index cbc0995..5bd767a 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1523,16 +1523,13 @@ qemuAgentGetVCPUs(qemuAgentPtr mon, return ret; }
-/** - * Set the VCPU state using guest agent. - * - * Returns -1 on error, ninfo in case everything was successful and less than - * ninfo on a partial failure. - */ -int -qemuAgentSetVCPUs(qemuAgentPtr mon, - qemuAgentCPUInfoPtr info, - size_t ninfo) + +/* returns the value provided by the guest agent or -1 on internal error */ +static int +qemuAgentSetVCPUsCommand(qemuAgentPtr mon, + qemuAgentCPUInfoPtr info, + size_t ninfo, + int *nmodified) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -1541,6 +1538,8 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, virJSONValuePtr cpu = NULL; size_t i;
+ *nmodified = 0; + /* create the key data array */ if (!(cpus = virJSONValueNewArray())) goto cleanup; @@ -1552,6 +1551,12 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, if (!(cpu = virJSONValueNewObject())) goto cleanup;
+ /* don't set state for cpus that were not touched */ + if (!in->modified) + continue;
This needs to go before the virJSONValueNewObject, otherwise it would leak memory.
+ + (*nmodified)++; + if (virJSONValueObjectAppendNumberInt(cpu, "logical-id", in->id) < 0) goto cleanup;
@@ -1564,6 +1569,11 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, cpu = NULL; }
+ if (*nmodified == 0) { + ret = 0; + goto cleanup; + } + if (!(cmd = qemuAgentMakeCommand("guest-set-vcpus", "a:vcpus", cpus, NULL))) @@ -1575,9 +1585,17 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) goto cleanup;
- if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + if (qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + /* All negative values are invalid. Return of 0 is bougs since we wouldn't
s/bougs/bogus/
+ * call the guest agent so that 0 cpus would be set successfully. Reporting + * more successfuly set vcpus that we've asked for is invalid */
s/successfuly/successfully/ s/invalid/invalid./
+ if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0 || + ret <= 0 || ret > *nmodified) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed return value")); + _("guest agent returned malformed or invalid return value")); + ret = -1; }
cleanup: @@ -1589,6 +1607,45 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, }
[...] ACK

Allow gathering available vcpu ids, their state and offlinability via the qemu guest agent. The maximum id was chosen arbitrarily and ought to be enough for everybody. --- src/qemu/qemu_driver.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 098840f..5b96ad8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -122,6 +122,8 @@ VIR_LOG_INIT("qemu.qemu_driver"); #define QEMU_SCHED_MIN_QUOTA 1000LL #define QEMU_SCHED_MAX_QUOTA 18446744073709551LL +#define QEMU_GUEST_VCPU_MAX_ID 4096 + #if HAVE_LINUX_KVM_H # include <linux/kvm.h> #endif @@ -19724,6 +19726,117 @@ static int qemuDomainRename(virDomainPtr dom, return ret; } + +static int +qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, + unsigned int *nparams, + qemuAgentCPUInfoPtr info, + int ninfo) +{ + virTypedParameterPtr par = NULL; + int npar = 0; + int maxpar = 0; + virBitmapPtr vcpus = NULL; + virBitmapPtr online = NULL; + virBitmapPtr offlinable = NULL; + char *tmp = NULL; + size_t i; + int ret = -1; + + if (!(vcpus = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID)) || + !(online = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID)) || + !(offlinable = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID))) + goto cleanup; + + for (i = 0; i < ninfo; i++) { + if (virBitmapSetBit(vcpus, info[i].id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("vcpu id '%u' reported by guest agent is out of " + "range"), info[i].id); + goto cleanup; + } + + if (info[i].online) + ignore_value(virBitmapSetBit(online, info[i].id)); + + if (info[i].offlinable) + ignore_value(virBitmapSetBit(offlinable, info[i].id)); + } + +#define ADD_BITMAP(name) \ + if (!(tmp = virBitmapFormat(name))) \ + goto cleanup; \ + if (virTypedParamsAddString(&par, &npar, &maxpar, #name, tmp) < 0) \ + goto cleanup; \ + VIR_FREE(tmp) + + ADD_BITMAP(vcpus); + ADD_BITMAP(online); + ADD_BITMAP(offlinable); + +#undef ADD_BITMAP + + *params = par; + *nparams = npar; + ret = 0; + + cleanup: + VIR_FREE(tmp); + virBitmapFree(vcpus); + virBitmapFree(online); + virBitmapFree(offlinable); + return ret; +} + + +static int +qemuDomainGetGuestVcpus(virDomainPtr dom, + virTypedParameterPtr *params, + unsigned int *nparams, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuAgentCPUInfoPtr info = NULL; + int ninfo = 0; + int ret = -1; + + virCheckFlags(0, ret); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainGetGuestVcpusEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endjob; + + qemuDomainObjEnterAgent(vm); + ninfo = qemuAgentGetVCPUs(qemuDomainGetAgent(vm), &info); + qemuDomainObjExitAgent(vm); + + if (ninfo < 0) + goto endjob; + + if (qemuDomainGetGuestVcpusParams(params, nparams, info, ninfo) < 0) + goto endjob; + + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + VIR_FREE(info); + virDomainObjEndAPI(&vm); + return ret; +} + + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectOpen = qemuConnectOpen, /* 0.2.0 */ @@ -19935,6 +20048,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSetUserPassword = qemuDomainSetUserPassword, /* 1.2.16 */ .domainRename = qemuDomainRename, /* 1.2.19 */ .domainMigrateStartPostCopy = qemuDomainMigrateStartPostCopy, /* 1.3.3 */ + .domainGetGuestVcpus = qemuDomainGetGuestVcpus, /* 2.0.0 */ }; -- 2.8.3

On Mon, Jun 20, 2016 at 04:34:20PM +0200, Peter Krempa wrote:
Allow gathering available vcpu ids, their state and offlinability via the qemu guest agent. The maximum id was chosen arbitrarily and ought to be enough for everybody. ---
[...]
+#define ADD_BITMAP(name) \ + if (!(tmp = virBitmapFormat(name))) \ + goto cleanup; \ + if (virTypedParamsAddString(&par, &npar, &maxpar, #name, tmp) < 0) \ + goto cleanup; \ + VIR_FREE(tmp) + + ADD_BITMAP(vcpus); + ADD_BITMAP(online); + ADD_BITMAP(offlinable); + +#undef ADD_BITMAP + + *params = par; + *nparams = npar;
You need to set par to NULL and to make it clear also npar to 0 and call virTypedParamsFree(par, npar) in the cleanup.
+ ret = 0; + + cleanup: + VIR_FREE(tmp); + virBitmapFree(vcpus); + virBitmapFree(online); + virBitmapFree(offlinable); + return ret; +}
ACK

Allow modification of specific vCPU states via the guest agent. --- src/qemu/qemu_driver.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5b96ad8..4bd8c92 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19837,6 +19837,88 @@ qemuDomainGetGuestVcpus(virDomainPtr dom, } +static int +qemuDomainSetGuestVcpus(virDomainPtr dom, + const char *cpumap, + int state, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virBitmapPtr map = NULL; + qemuAgentCPUInfoPtr info = NULL; + int ninfo = 0; + size_t i; + int ret = -1; + + virCheckFlags(0, -1); + + if (state != 0 && state != 1) { + virReportInvalidArg(state, "%s", _("unsupported state value")); + return -1; + } + + if (virBitmapParse(cpumap, &map, 4096) < 0) + goto cleanup; + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainSetGuestVcpusEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endjob; + + qemuDomainObjEnterAgent(vm); + ninfo = qemuAgentGetVCPUs(qemuDomainGetAgent(vm), &info); + qemuDomainObjExitAgent(vm); + + if (ninfo < 0) + goto endjob; + + for (i = 0; i < ninfo; i++) { + if (!virBitmapIsBitSet(map, info[i].id)) + continue; + + if (!state && !info[i].offlinable) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("vCPU '%u' is not offlinable"), info[i].id); + goto endjob; + } + + info[i].online = !!state; + info[i].modified = true; + + ignore_value(virBitmapClearBit(map, info[i].id)); + } + + if (!virBitmapIsAllClear(map)) { + char *tmp = virBitmapFormat(map); + virReportError(VIR_ERR_INVALID_ARG, + _("guest is missing vCPUs '%s'"), NULLSTR(tmp)); + VIR_FREE(tmp); + goto endjob; + } + + qemuDomainObjEnterAgent(vm); + ret = qemuAgentSetVCPUs(qemuDomainGetAgent(vm), info, ninfo); + qemuDomainObjExitAgent(vm); + + endjob: + qemuDomainObjEndJob(driver, vm); + + cleanup: + VIR_FREE(info); + virBitmapFree(map); + virDomainObjEndAPI(&vm); + return ret; +} + + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectOpen = qemuConnectOpen, /* 0.2.0 */ @@ -20049,6 +20131,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainRename = qemuDomainRename, /* 1.2.19 */ .domainMigrateStartPostCopy = qemuDomainMigrateStartPostCopy, /* 1.3.3 */ .domainGetGuestVcpus = qemuDomainGetGuestVcpus, /* 2.0.0 */ + .domainSetGuestVcpus = qemuDomainSetGuestVcpus, /* 2.0.0 */ }; -- 2.8.3

On Mon, Jun 20, 2016 at 04:34:21PM +0200, Peter Krempa wrote:
Allow modification of specific vCPU states via the guest agent. --- src/qemu/qemu_driver.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5b96ad8..4bd8c92 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19837,6 +19837,88 @@ qemuDomainGetGuestVcpus(virDomainPtr dom, }
+static int +qemuDomainSetGuestVcpus(virDomainPtr dom, + const char *cpumap, + int state, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virBitmapPtr map = NULL; + qemuAgentCPUInfoPtr info = NULL; + int ninfo = 0; + size_t i; + int ret = -1; + + virCheckFlags(0, -1); + + if (state != 0 && state != 1) { + virReportInvalidArg(state, "%s", _("unsupported state value")); + return -1; + } + + if (virBitmapParse(cpumap, &map, 4096) < 0) + goto cleanup;
s/4096/QEMU_GUEST_VCPU_MAX_ID/
+ + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainSetGuestVcpusEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endjob;
[...] ACK
participants (2)
-
Pavel Hrdina
-
Peter Krempa