Re: [libvirt] [PATCH v2 2/5] remote: Implement the remote protocol for virDomainGetFSInfo

On 11/17/2014 04:26 PM, Tomoki Sekiyama wrote: > Add daemon and driver code to (de-)serialize virDomainFSInfo. > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama(a)hds.com&gt; > --- > daemon/remote.c | 117 >++++++++++++++++++++++++++++++++++++++++++ > src/remote/remote_driver.c | 92 +++++++++++++++++++++++++++++++++ > src/remote/remote_protocol.x | 32 +++++++++++ > src/remote_protocol-structs | 21 ++++++++ > src/rpc/gendispatch.pl | 1 > 5 files changed, 262 insertions(+), 1 deletion(-) > > diff --git a/daemon/remote.c b/daemon/remote.c > index 1d7082e..9b89fd0 100644 > --- a/daemon/remote.c > +++ b/daemon/remote.c > @@ -6336,6 +6336,123 @@ remoteDispatchNodeAllocPages(virNetServerPtr >server ATTRIBUTE_UNUSED, > } > > > +static int > +remoteDispatchDomainGetFSInfo(virNetServerPtr server ATTRIBUTE_UNUSED, > + virNetServerClientPtr client, > + virNetMessagePtr msg ATTRIBUTE_UNUSED, > + virNetMessageErrorPtr rerr, > + remote_domain_get_fsinfo_args *args, > + remote_domain_get_fsinfo_ret *ret) Are we sure we have to write this by hand? [1] > @@ -8171,6 +8262,7 @@ static virHypervisorDriver hypervisor_driver = { > .connectGetDomainCapabilities = >remoteConnectGetDomainCapabilities, /* 1.2.7 */ > .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* >1.2.8 */ > .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */ > + .domainGetFSInfo = remoteDomainGetFSInfo, /* 1.2.10 */ 1.2.10 is wrong; the next release is 1.2.11. > }; > > static virNetworkDriver network_driver = { > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x > index ebf4530..10c8068 100644 > --- a/src/remote/remote_protocol.x > +++ b/src/remote/remote_protocol.x > @@ -250,6 +250,12 @@ const REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX = >4096; > /* Upper limit of message size for tunable event. */ > const REMOTE_DOMAIN_EVENT_TUNABLE_MAX = 2048; > > +/* Upper limit on number of mountpoints in fsinfo */ > +const REMOTE_DOMAIN_FSINFO_MAX = 256; > + > +/* Upper limit on number of disks per mountpoint in fsinfo */ > +const REMOTE_DOMAIN_FSINFO_DISKS_MAX = 256; > + > /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ > typedef opaque remote_uuid[VIR_UUID_BUFLEN]; > > @@ -3111,6 +3117,24 @@ struct remote_connect_get_all_domain_stats_args { > struct remote_connect_get_all_domain_stats_ret { > remote_domain_stats_record retStats<REMOTE_DOMAIN_LIST_MAX>; > }; > + > +struct remote_domain_fsinfo { > + remote_nonnull_string mountpoint; > + remote_nonnull_string name; > + remote_nonnull_string type; > + remote_nonnull_string dev_aliases<REMOTE_DOMAIN_FSINFO_DISKS_MAX>; >/* (const char **) */ Can any of these values ever be NULL because the guest didn't provide them? If so, you want remote_string instead of remote_nonnull_string. It wasn't obvious to me in the 1/5 documentation whether values are guaranteed to be non-NULL.

> + > + /** > + * @generate: none > + * @acl: domain:read > + */ > + REMOTE_PROC_DOMAIN_GET_FSINFO = 348 [1] Did you try any other values of @generate to see if things get transferred correctly without writing it by hand? Then again, looking at the structure you are transferring, it consists of mallocing an array of structures which themselves malloc an array of names, so I guess you are right that you have to manage it by hand (the generator probably doesn't do that).