From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54943) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsGCP-0003ZE-R4 for qemu-devel@nongnu.org; Tue, 12 May 2015 15:50:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsGCL-0001dQ-D6 for qemu-devel@nongnu.org; Tue, 12 May 2015 15:50:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47402) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsGCL-0001dI-4t for qemu-devel@nongnu.org; Tue, 12 May 2015 15:50:37 -0400 Message-ID: <5552598A.30701@redhat.com> Date: Tue, 12 May 2015 13:50:34 -0600 From: Eric Blake MIME-Version: 1.0 References: <1431457421-31877-1-git-send-email-dslutz@verizon.com> <1431457421-31877-5-git-send-email-dslutz@verizon.com> In-Reply-To: <1431457421-31877-5-git-send-email-dslutz@verizon.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Wofn31X2t3Lt7882U4Iba0ljj7i6LxMXm" Subject: Re: [Qemu-devel] [PATCH v6 4/7] vmport_rpc: Add QMP access to vmport_rpc object. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Don Slutz , qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" , Markus Armbruster , Luiz Capitulino , Anthony Liguori , Paolo Bonzini , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Richard Henderson This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Wofn31X2t3Lt7882U4Iba0ljj7i6LxMXm Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/12/2015 01:03 PM, Don Slutz wrote: > This adds one new inject command: >=20 > inject-vmport-action >=20 > And three guest info commands: >=20 > vmport-guestinfo-set > vmport-guestinfo-get > query-vmport-guestinfo >=20 > More details in qmp-commands.hx >=20 > Signed-off-by: Don Slutz > --- > +/* > + * Run func() for every VMPortRpc device, traverse the tree for > + * everything else. Note: This routine expects that opaque is a > + * VMPortRpcFind pointer and not NULL. > + */ > +static int find_VMPortRpc_device(Object *obj, void *opaque) > +{ > + VMPortRpcFind *find =3D opaque; > + Object *dev; > + VMPortRpcState *s; > + > + if (find->found) { Why not assert(find) instead of leaving it to the comment? > +/* > + * Loop through all dynamically created VMPortRpc devices and call > + * func() for each instance. > + */ > +static int foreach_dynamic_vmport_rpc_device(FindVMPortRpcDeviceFunc *= func, > + void *arg) > +{ > + VMPortRpcFind find =3D { > + .func =3D func, > + .arg =3D arg, Is it worth marking arg const here and in the VMPortRpcFind struct... > +void qmp_inject_vmport_action(enum VmportAction action, Error **errp) > +{ > + int rc; > + > + switch (action) { > + case VMPORT_ACTION_REBOOT: > + rc =3D foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,= > + (void *)"OS_Reboot"); =2E..so that you don't have to cast away const here? > + break; > + case VMPORT_ACTION_HALT: > + rc =3D foreach_dynamic_vmport_rpc_device(vmport_rpc_find_send,= > + (void *)"OS_Halt"); > + break; > + case VMPORT_ACTION_MAX: > + assert(action !=3D VMPORT_ACTION_MAX); > + rc =3D 0; /* Should be impossible to get here. */ I'd rather abort() if someone compiled with -NDEBUG. > + break; > + } > + convert_local_rc(errp, rc); > +} > + > +typedef struct keyValue { > + void *key_data; > + void *value_data; > + unsigned int key_len; > + unsigned int value_len; Should these be size_t? > +void qmp_vmport_guestinfo_set(const char *key, const char *value, > + bool has_format, enum DataFormat format,= > + Error **errp) > +{ > + int rc; > + keyValue key_value; > + > + if (strncmp(key, "guestinfo.", strlen("guestinfo.")) =3D=3D 0) { > + key_value.key_data =3D (void *)(key + strlen("guestinfo.")); > + key_value.key_len =3D strlen(key) - strlen("guestinfo."); > + } else { > + key_value.key_data =3D (void *)key; Casting to (void*) looks awkward; should key_data should be typed 'const void *' to avoid the need for a cast? For that matter, why is it void*, why not 'const char *'? > +++ b/qmp-commands.hx > @@ -490,6 +490,126 @@ Note: inject-nmi fails when the guest doesn't sup= port injecting. > EQMP > +SQMP > +vmport-guestinfo-set > +---------- Still mismatches on ---- line length (several sites). > +-> { "execute": "vmport-guestinfo-get", > + "arguments": { "key": "guestinfo.foo", > + "format": "utf8" } } > +<- {"return": {"value": "abcdefgh"}} > + > + > +EQMP > + > + { > + .name =3D "query-vmport-guestinfo", > + .args_type =3D "", > + .mhandler.cmd_new =3D qmp_marshal_input_query_vmport_guestinfo= , > + }, > + > +SQMP > +query-vmport-guestinfo > +---------- > + > +Returns information about VMWare Tools guestinfo. The returned value = is a json-array > +of all keys. > + > +Example: > + > +-> { "execute": "query-vmport-guestinfo" } > +<- { > + "return": [ > + { > + "key": "guestinfo.ip" > + }, So if I'm following this correctly, a user has to call query-vmport-guestinfo to learn the key names, and then once per key call vmport-guetsinfo-get to learn the key values. Why not just return key names and values all at once, to save the multiple call overhead? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Wofn31X2t3Lt7882U4Iba0ljj7i6LxMXm Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVUlmKAAoJEKeha0olJ0Nq0mMH/R78h3ci6nFP9gSgbXHSKpxC NoLhLZTkssqiXKDbujbENlmmFAkVJieFuIei2TNeu8G1bd/WroxvLjpHKCzIQAVI CBBlZ7DzpPfG2btmcv6JVGOUfR9023JW9eIZ0Db7hf1ug3i4oCZVb6rsG3agQm1C 0l2Xa89TbIe24kbg3UWbnpmE2eoGEr7LxAMdzJ9M3KnSe2UJB1yB1xaYvTUIjew4 CBQ+EV9HKnejKF7ehA/+CtLE65DSQUxImj8inyggF5WfhX+zCB5CxRj03vvlzVkw 87een1Dwh8/t4lO5Bi4pKVyVJhe/WD/REsMrCDtY/QVBrDNIlTRne2R72kvnDDk= =hLoM -----END PGP SIGNATURE----- --Wofn31X2t3Lt7882U4Iba0ljj7i6LxMXm--