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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
> ---
>  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).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]