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

[Tomoki Sekiyama]

On 11/20/14, 14:12 , "Eric Blake" <eblake at redhat.com> wrote:

>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 at hds.com>
>> ---
>>  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.

These are always filled. If guest agent doesn¹t pass these values,
the qemu driver will return an error.

>> +
>> +    /**
>> +     * @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).

Right, I¹ve tried code generation, but it couldn¹t handle dynamically
allocated arrays in an array...