[libvirt] [PATCH 0/5]: Add a API for query domain disk sizing info

This is somewhat late for the 0.8.1 release, but this is a rather critical API for some of the use cases of RHEV. It introduces a new API against a virDomainPtr to allow the direct querying of the size of guest block devices. This is modelled on the virStorageVol API for getting size, so I'm pretty confident on the design and impl here.

Some applications need to be able to query a guest's disk info, even for paths not managed by the storage pool APIs. This adds a very simple API to get this information, modelled on the virStorageVolGetInfo API, but with an extra field 'physical'. Normally 'physical' and 'allocation' will be identical, but in the case of a qcow2-like file stored inside a block device 'physical' will give the block device size, while 'allocation' will give the qcow2 image size * include/libvirt/libvirt.h.in: Define virDomainGetBlockInfo --- include/libvirt/libvirt.h.in | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 4addc62..3ce59cf 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -733,6 +733,42 @@ int virDomainBlockPeek (virDomainPtr dom, void *buffer, unsigned int flags); + +/** virDomainBlockInfo: + * + * This struct provides information about the size of a block device backing store + * + * Examples: + * + * - Fully allocated raw file in filesystem: + * * capacity, allocation, physical: All the same + * + * - Sparse raw file in filesystem: + * * capacity: logical size of the file + * * allocation, physical: number of blocks allocated to file + * + * - qcow2 file in filesystem + * * capacity: logical size from qcow2 header + * * allocation, physical: logical size of the file / highest qcow extent (identical) + * + * - qcow2 file in a block device + * * capacity: logical size from qcow2 header + * * allocation: highest qcow extent written + * * physical: size of the block device container + */ +typedef struct _virDomainBlockInfo virDomainBlockInfo; +typedef virDomainBlockInfo *virDomainBlockInfoPtr; +struct _virDomainBlockInfo { + unsigned long long capacity; /* logical size of the block device backing image */ + unsigned long long allocation; /* allocated extent of the block device backing image */ + unsigned long long physical; /* physical size of the container of the backing image */ +}; + +int virDomainGetBlockInfo(virDomainPtr dom, + const char *path, + virDomainBlockInfoPtr info, + unsigned int flags); + /* Memory peeking flags. */ typedef enum { VIR_MEMORY_VIRTUAL = 1, /* addresses are virtual addresses */ -- 1.6.6.1

On 04/27/2010 01:34 PM, Daniel P. Berrange wrote:
Some applications need to be able to query a guest's disk info, even for paths not managed by the storage pool APIs. This adds a very simple API to get this information, modelled on the virStorageVolGetInfo API, but with an extra field 'physical'. Normally 'physical' and 'allocation' will be identical, but in the case of a qcow2-like file stored inside a block device 'physical' will give the block device size, while 'allocation' will give the qcow2 image size
ACK, and thanks for the examples in the comments.
+ * - Fully allocated raw file in filesystem: + * * capacity, allocation, physical: All the same
Actually, might it be possible that allocation/physical might be slightly higher than capacity, to account for inode overhead and rounding up to block size? But I don't see the need to clutter this documentation with that level of detail. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Tue, Apr 27, 2010 at 04:16:24PM -0600, Eric Blake wrote:
On 04/27/2010 01:34 PM, Daniel P. Berrange wrote:
Some applications need to be able to query a guest's disk info, even for paths not managed by the storage pool APIs. This adds a very simple API to get this information, modelled on the virStorageVolGetInfo API, but with an extra field 'physical'. Normally 'physical' and 'allocation' will be identical, but in the case of a qcow2-like file stored inside a block device 'physical' will give the block device size, while 'allocation' will give the qcow2 image size
ACK, and thanks for the examples in the comments.
+ * - Fully allocated raw file in filesystem: + * * capacity, allocation, physical: All the same
Actually, might it be possible that allocation/physical might be slightly higher than capacity, to account for inode overhead and rounding up to block size? But I don't see the need to clutter this documentation with that level of detail.
Yeah, there's a tiny difference, but if you squint with your eyes they are the same :-P Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

This defines the internal driver API and stubs out each driver * src/driver.h: Define virDrvDomainGetBlockInfo signature * src/libvirt.c, src/libvirt_public.syms: Glue public API to drivers * src/esx/esx_driver.c, src/lxc/lxc_driver.c, src/opennebula/one_driver.c, src/openvz/openvz_driver.c, src/phyp/phyp_driver.c, src/test/test_driver.c, src/uml/uml_driver.c, src/vbox/vbox_tmpl.c, src/xen/xen_driver.c, src/xenapi/xenapi_driver.c: Stub out driver --- src/driver.h | 7 ++++++ src/esx/esx_driver.c | 1 + src/libvirt.c | 48 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + 13 files changed, 70 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index f8db9c1..0975b59 100644 --- a/src/driver.h +++ b/src/driver.h @@ -261,6 +261,12 @@ typedef int unsigned long long start, size_t size, void *buffer, unsigned int flags); +typedef int + (*virDrvDomainGetBlockInfo) + (virDomainPtr domain, + const char *path, + virDomainBlockInfoPtr info, + unsigned int flags); typedef int (*virDrvDomainMigratePrepare) @@ -525,6 +531,7 @@ struct _virDriver { virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; + virDrvDomainGetBlockInfo domainGetBlockInfo; virDrvNodeGetCellsFreeMemory nodeGetCellsFreeMemory; virDrvNodeGetFreeMemory getFreeMemory; virDrvDomainEventRegister domainEventRegister; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index c0c3195..8e55fc6 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3776,6 +3776,7 @@ static virDriver esxDriver = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ NULL, /* nodeGetCellsFreeMemory */ esxNodeGetFreeMemory, /* nodeGetFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/libvirt.c b/src/libvirt.c index ff36681..7760f10 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -4625,6 +4625,54 @@ error: } +/** + * virDomainGetBlockInfo: + * @domain: a domain object + * @path: path to the block device or file + * @info: pointer to a virDomainBlockInfo structure allocated by the user + * + * Extract information about a domain's block device. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainGetBlockInfo(virDomainPtr domain, const char *path, virDomainBlockInfoPtr info, unsigned int flags) +{ + virConnectPtr conn; + DEBUG("domain=%p, info=%p", domain, info); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return (-1); + } + if (info == NULL) { + virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + memset(info, 0, sizeof(virDomainBlockInfo)); + + conn = domain->conn; + + if (conn->driver->domainGetBlockInfo) { + int ret; + ret = conn->driver->domainGetBlockInfo (domain, path, info, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + + /************************************************************************ * * * Handling of defined but not running domains * diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index b4db904..81465d3 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -394,4 +394,9 @@ LIBVIRT_0.8.0 { } LIBVIRT_0.7.7; +LIBVIRT_0.8.1 { + global: + virDomainGetBlockInfo; +} LIBVIRT_0.8.0; + # .... define new API here using predicted next version number .... diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 2851a2a..409b1cf 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2517,6 +2517,7 @@ static virDriver lxcDriver = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ lxcDomainEventRegister, /* domainEventRegister */ diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index cdd61eb..acd52c2 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -771,6 +771,7 @@ static virDriver oneDriver = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 47004d6..00b8a14 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1525,6 +1525,7 @@ static virDriver openvzDriver = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index f4d817e..467ea19 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1630,6 +1630,7 @@ virDriver phypDriver = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4ea0279..6706cba 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5280,6 +5280,7 @@ static virDriver testDriver = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ testNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ testDomainEventRegister, /* domainEventRegister */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index a251e89..644ac8b 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1911,6 +1911,7 @@ static virDriver umlDriver = { NULL, /* domainMemoryStats */ umlDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index f564213..6a9a2bf 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8164,6 +8164,7 @@ virDriver NAME(Driver) = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ #if VBOX_API_VERSION == 2002 diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index b6dcf8d..91f0acd 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1961,6 +1961,7 @@ static virDriver xenUnifiedDriver = { NULL, /* domainMemoryStats */ xenUnifiedDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ xenUnifiedNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ xenUnifiedNodeGetFreeMemory, /* getFreeMemory */ xenUnifiedDomainEventRegister, /* domainEventRegister */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 75796d6..7ef03cb 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1762,6 +1762,7 @@ static virDriver xenapiDriver = { NULL, /* domainMemoryStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ + NULL, /* domainGetBlockInfo */ xenapiNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ xenapiNodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ -- 1.6.6.1

On 04/27/2010 01:34 PM, Daniel P. Berrange wrote:
This defines the internal driver API and stubs out each driver
* src/driver.h: Define virDrvDomainGetBlockInfo signature * src/libvirt.c, src/libvirt_public.syms: Glue public API to drivers * src/esx/esx_driver.c, src/lxc/lxc_driver.c, src/opennebula/one_driver.c, src/openvz/openvz_driver.c, src/phyp/phyp_driver.c, src/test/test_driver.c, src/uml/uml_driver.c, src/vbox/vbox_tmpl.c, src/xen/xen_driver.c, src/xenapi/xenapi_driver.c: Stub out driver ... +/** + * virDomainGetBlockInfo: + * @domain: a domain object + * @path: path to the block device or file + * @info: pointer to a virDomainBlockInfo structure allocated by the user
Document that flags is 0 for now?
+ * + * Extract information about a domain's block device. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainGetBlockInfo(virDomainPtr domain, const char *path, virDomainBlockInfoPtr info, unsigned int flags) +{ + virConnectPtr conn; + DEBUG("domain=%p, info=%p", domain, info);
List flags in the DEBUG. ACK with those nits addressed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* daemon/remote.c: Server side dispatcher * daemon/remote_dispatch_args.h, daemon/remote_dispatch_prototypes.h, daemon/remote_dispatch_ret.h, daemon/remote_dispatch_table.h: Update with new API * src/remote/remote_driver.c: Client side dispatcher * src/remote/remote_protocol.c, src/remote/remote_protocol.h: Update * src/remote/remote_protocol.x: Define new wire protocol --- daemon/remote.c | 34 ++++++++++++++++++++++++++++++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++++++++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 +++++ src/remote/remote_driver.c | 35 +++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.c | 26 ++++++++++++++++++++++++++ src/remote/remote_protocol.h | 19 +++++++++++++++++++ src/remote/remote_protocol.x | 15 ++++++++++++++- 9 files changed, 143 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 98abd33..6dbe7ee 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6434,6 +6434,40 @@ remoteDispatchNumOfNwfilters (struct qemud_server *server ATTRIBUTE_UNUSED, } +static int +remoteDispatchDomainGetBlockInfo (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_domain_get_block_info_args *args, + remote_domain_get_block_info_ret *ret) +{ + virDomainPtr dom; + virDomainBlockInfo info; + + dom = get_nonnull_domain (conn, args->dom); + if (dom == NULL) { + remoteDispatchConnError(rerr, conn); + return -1; + } + + if (virDomainGetBlockInfo (dom, args->path, &info, args->flags) == -1) { + virDomainFree(dom); + remoteDispatchConnError(rerr, conn); + return -1; + } + + ret->capacity = info.capacity; + ret->allocation = info.allocation; + ret->physical = info.physical; + + virDomainFree(dom); + + return 0; +} + + /*----- Helpers. -----*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h index 85ebedf..b0c52d2 100644 --- a/daemon/remote_dispatch_args.h +++ b/daemon/remote_dispatch_args.h @@ -163,3 +163,4 @@ remote_domain_snapshot_current_args val_remote_domain_snapshot_current_args; remote_domain_revert_to_snapshot_args val_remote_domain_revert_to_snapshot_args; remote_domain_snapshot_delete_args val_remote_domain_snapshot_delete_args; + remote_domain_get_block_info_args val_remote_domain_get_block_info_args; diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h index f6fcff8..79d1eec 100644 --- a/daemon/remote_dispatch_prototypes.h +++ b/daemon/remote_dispatch_prototypes.h @@ -210,6 +210,14 @@ static int remoteDispatchDomainGetAutostart( remote_error *err, remote_domain_get_autostart_args *args, remote_domain_get_autostart_ret *ret); +static int remoteDispatchDomainGetBlockInfo( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *err, + remote_domain_get_block_info_args *args, + remote_domain_get_block_info_ret *ret); static int remoteDispatchDomainGetInfo( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/remote_dispatch_ret.h b/daemon/remote_dispatch_ret.h index 895b511..f9d6237 100644 --- a/daemon/remote_dispatch_ret.h +++ b/daemon/remote_dispatch_ret.h @@ -133,3 +133,4 @@ remote_domain_snapshot_lookup_by_name_ret val_remote_domain_snapshot_lookup_by_name_ret; remote_domain_has_current_snapshot_ret val_remote_domain_has_current_snapshot_ret; remote_domain_snapshot_current_ret val_remote_domain_snapshot_current_ret; + remote_domain_get_block_info_ret val_remote_domain_get_block_info_ret; diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h index ebe8da4..984634f 100644 --- a/daemon/remote_dispatch_table.h +++ b/daemon/remote_dispatch_table.h @@ -972,3 +972,8 @@ .args_filter = (xdrproc_t) xdr_remote_domain_snapshot_delete_args, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* DomainGetBlockInfo => 194 */ + .fn = (dispatch_fn) remoteDispatchDomainGetBlockInfo, + .args_filter = (xdrproc_t) xdr_remote_domain_get_block_info_args, + .ret_filter = (xdrproc_t) xdr_remote_domain_get_block_info_ret, +}, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1917f26..e589f8e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3622,6 +3622,40 @@ done: } static int +remoteDomainGetBlockInfo (virDomainPtr domain, + const char *path, + virDomainBlockInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_block_info_args args; + remote_domain_get_block_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.path = (char*)path; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_INFO, + (xdrproc_t) xdr_remote_domain_get_block_info_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_block_info_ret, (char *) &ret) == -1) + goto done; + + info->allocation = ret.allocation; + info->capacity = ret.capacity; + info->physical = ret.physical; + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainManagedSave (virDomainPtr domain, unsigned int flags) { int rv = -1; @@ -10144,6 +10178,7 @@ static virDriver remote_driver = { remoteDomainMemoryStats, /* domainMemoryStats */ remoteDomainBlockPeek, /* domainBlockPeek */ remoteDomainMemoryPeek, /* domainMemoryPeek */ + remoteDomainGetBlockInfo, /* domainGetBlockInfo */ remoteNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ remoteNodeGetFreeMemory, /* getFreeMemory */ remoteDomainEventRegister, /* domainEventRegister */ diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index eb0f9c3..71eed42 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -726,6 +726,32 @@ xdr_remote_domain_memory_peek_ret (XDR *xdrs, remote_domain_memory_peek_ret *obj } bool_t +xdr_remote_domain_get_block_info_args (XDR *xdrs, remote_domain_get_block_info_args *objp) +{ + + if (!xdr_remote_nonnull_domain (xdrs, &objp->dom)) + return FALSE; + if (!xdr_remote_nonnull_string (xdrs, &objp->path)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; + return TRUE; +} + +bool_t +xdr_remote_domain_get_block_info_ret (XDR *xdrs, remote_domain_get_block_info_ret *objp) +{ + + if (!xdr_uint64_t (xdrs, &objp->allocation)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->capacity)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->physical)) + return FALSE; + return TRUE; +} + +bool_t xdr_remote_list_domains_args (XDR *xdrs, remote_list_domains_args *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index 92fd6df..8c4291f 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -389,6 +389,20 @@ struct remote_domain_memory_peek_ret { }; typedef struct remote_domain_memory_peek_ret remote_domain_memory_peek_ret; +struct remote_domain_get_block_info_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + u_int flags; +}; +typedef struct remote_domain_get_block_info_args remote_domain_get_block_info_args; + +struct remote_domain_get_block_info_ret { + uint64_t allocation; + uint64_t capacity; + uint64_t physical; +}; +typedef struct remote_domain_get_block_info_ret remote_domain_get_block_info_ret; + struct remote_list_domains_args { int maxids; }; @@ -2191,6 +2205,7 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_CURRENT = 191, REMOTE_PROC_DOMAIN_REVERT_TO_SNAPSHOT = 192, REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193, + REMOTE_PROC_DOMAIN_GET_BLOCK_INFO = 194, }; typedef enum remote_procedure remote_procedure; @@ -2277,6 +2292,8 @@ extern bool_t xdr_remote_domain_block_peek_args (XDR *, remote_domain_block_pee extern bool_t xdr_remote_domain_block_peek_ret (XDR *, remote_domain_block_peek_ret*); extern bool_t xdr_remote_domain_memory_peek_args (XDR *, remote_domain_memory_peek_args*); extern bool_t xdr_remote_domain_memory_peek_ret (XDR *, remote_domain_memory_peek_ret*); +extern bool_t xdr_remote_domain_get_block_info_args (XDR *, remote_domain_get_block_info_args*); +extern bool_t xdr_remote_domain_get_block_info_ret (XDR *, remote_domain_get_block_info_ret*); extern bool_t xdr_remote_list_domains_args (XDR *, remote_list_domains_args*); extern bool_t xdr_remote_list_domains_ret (XDR *, remote_list_domains_ret*); extern bool_t xdr_remote_num_of_domains_ret (XDR *, remote_num_of_domains_ret*); @@ -2607,6 +2624,8 @@ extern bool_t xdr_remote_domain_block_peek_args (); extern bool_t xdr_remote_domain_block_peek_ret (); extern bool_t xdr_remote_domain_memory_peek_args (); extern bool_t xdr_remote_domain_memory_peek_ret (); +extern bool_t xdr_remote_domain_get_block_info_args (); +extern bool_t xdr_remote_domain_get_block_info_ret (); extern bool_t xdr_remote_list_domains_args (); extern bool_t xdr_remote_list_domains_ret (); extern bool_t xdr_remote_num_of_domains_ret (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 60f93b2..ae306d2 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -484,6 +484,18 @@ struct remote_domain_memory_peek_ret { opaque buffer<REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX>; }; +struct remote_domain_get_block_info_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned flags; +}; + +struct remote_domain_get_block_info_ret { + unsigned hyper allocation; + unsigned hyper capacity; + unsigned hyper physical; +}; + struct remote_list_domains_args { int maxids; }; @@ -1983,7 +1995,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_CURRENT = 191, REMOTE_PROC_DOMAIN_REVERT_TO_SNAPSHOT = 192, - REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193 + REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193, + REMOTE_PROC_DOMAIN_GET_BLOCK_INFO = 194 /* * Notice how the entries are grouped in sets of 10 ? -- 1.6.6.1

On 04/27/2010 01:35 PM, Daniel P. Berrange wrote:
* daemon/remote.c: Server side dispatcher * daemon/remote_dispatch_args.h, daemon/remote_dispatch_prototypes.h, daemon/remote_dispatch_ret.h, daemon/remote_dispatch_table.h: Update with new API * src/remote/remote_driver.c: Client side dispatcher * src/remote/remote_protocol.c, src/remote/remote_protocol.h: Update * src/remote/remote_protocol.x: Define new wire protocol
Looks pretty straight-forward. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* src/qemu/qemu_driver.c: Implementation of virDomainGetBlockInfo * src/util/storage_file.h: Add DEV_BSIZE * src/storage/storage_backend.c: Remove DEV_BSIZE --- src/qemu/qemu_driver.c | 116 +++++++++++++++++++++++++++++++++++++++++ src/storage/storage_backend.c | 4 -- src/util/storage_file.h | 4 ++ 3 files changed, 120 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c4876e..7bc2906 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -84,6 +84,7 @@ #include "macvtap.h" #include "nwfilter/nwfilter_gentech_driver.h" #include "hooks.h" +#include "storage_file.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -8893,6 +8894,120 @@ cleanup: } +static int qemuDomainGetBlockInfo(virDomainPtr dom, + const char *path, + virDomainBlockInfoPtr info, + unsigned int flags) { + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + int fd = -1; + off_t end; + virStorageFileMetadata meta; + struct stat sb; + int i; + + virCheckFlags(0, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!path || path[0] == '\0') { + qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("NULL or empty path")); + goto cleanup; + } + + /* Check the path belongs to this domain. */ + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (vm->def->disks[i]->src != NULL && + STREQ (vm->def->disks[i]->src, path)) { + ret = 0; + break; + } + } + + if (ret != 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("invalid path %s not assigned to domain"), path); + goto cleanup; + } + + ret = -1; + + /* The path is correct, now try to open it and get its size. */ + fd = open (path, O_RDONLY); + if (fd == -1) { + virReportSystemError(errno, + _("failed to open path '%s'"), path); + goto cleanup; + } + + /* Probe for magic formats */ + memset(&meta, 0, sizeof(meta)); + if (virStorageFileGetMetadataFromFD(path, fd, &meta) < 0) + goto cleanup; + + /* Get info for normal formats */ + if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, + _("cannot stat file '%s'"), path); + goto cleanup; + } + + if (S_ISREG(sb.st_mode)) { +#ifndef __MINGW32__ + info->physical = (unsigned long long)sb.st_blocks * + (unsigned long long)DEV_BSIZE; +#else + info->physical = sb.st_size; +#endif + /* Regular files may be sparse, so logical size (capacity) is not same + * as actual physical above + */ + info->capacity = sb.st_size; + } else { + /* NB. Because we configure with AC_SYS_LARGEFILE, off_t should + * be 64 bits on all platforms. + */ + end = lseek (fd, 0, SEEK_END); + if (end == (off_t)-1) { + virReportSystemError(errno, + _("failed to seek to end of %s"), path); + goto cleanup; + } + info->physical = end; + info->capacity = end; + } + + /* If the file we probed has a capacity set, then override + * what we calculated from file/block extents */ + if (meta.capacity) + info->capacity = meta.capacity; + + /* XXX allocation will need to be pulled from QEMU for + * the qcow inside LVM case */ + info->allocation = info->physical; + + ret = 0; + +cleanup: + if (fd != -1) + close(fd); + if (vm) + virDomainObjUnlock(vm); + return ret; +} + + static int qemuDomainEventRegister(virConnectPtr conn, virConnectDomainEventCallback callback, @@ -11335,6 +11450,7 @@ static virDriver qemuDriver = { qemudDomainMemoryStats, /* domainMemoryStats */ qemudDomainBlockPeek, /* domainBlockPeek */ qemudDomainMemoryPeek, /* domainMemoryPeek */ + qemuDomainGetBlockInfo, /* domainGetBlockInfo */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ nodeGetFreeMemory, /* getFreeMemory */ qemuDomainEventRegister, /* domainEventRegister */ diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f0074ed..5003b8c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -73,10 +73,6 @@ # include "storage_backend_fs.h" #endif -#ifndef DEV_BSIZE -# define DEV_BSIZE 512 -#endif - #define VIR_FROM_THIS VIR_FROM_STORAGE static virStorageBackendPtr backends[] = { diff --git a/src/util/storage_file.h b/src/util/storage_file.h index ef97100..deb8c79 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -51,6 +51,10 @@ typedef struct _virStorageFileMetadata { bool encrypted; } virStorageFileMetadata; +#ifndef DEV_BSIZE +# define DEV_BSIZE 512 +#endif + int virStorageFileGetMetadata(const char *path, virStorageFileMetadata *meta); int virStorageFileGetMetadataFromFD(const char *path, -- 1.6.6.1

On 04/27/2010 01:35 PM, Daniel P. Berrange wrote:
* src/qemu/qemu_driver.c: Implementation of virDomainGetBlockInfo * src/util/storage_file.h: Add DEV_BSIZE * src/storage/storage_backend.c: Remove DEV_BSIZE
+ if (S_ISREG(sb.st_mode)) { +#ifndef __MINGW32__ + info->physical = (unsigned long long)sb.st_blocks * + (unsigned long long)DEV_BSIZE;
Is it worth checking sb.st_blksize rather than DEV_BSIZE on XSI systems where that is part of struct stat? In particular, POSIX allows the block size to be file-system dependent, and some file systems (like NTFS) have 4k rather than 512 as the block size (is anyone daring enough to use NTFS for raw file storage?).
+#ifndef DEV_BSIZE +# define DEV_BSIZE 512 +#endif
Maybe the right thing to do is rewrite a macro: #if HAVE_STRUCT_STAT_ST_BLKSIZE # define BSIZE(s) ((s).st_blksize) #elif defined DEV_BSIZE # define BSIZE(s) DEV_BSIZE #else # define BSIZE(s) 512 #endif and use BSIZE(sb) instead of DEV_BSIZE; it also involves using a configure check for AC_CHECK_MEMBERS([struct stat.st_blksize]). But I think that can be an independent patch, so: ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Tue, Apr 27, 2010 at 04:43:08PM -0600, Eric Blake wrote:
On 04/27/2010 01:35 PM, Daniel P. Berrange wrote:
* src/qemu/qemu_driver.c: Implementation of virDomainGetBlockInfo * src/util/storage_file.h: Add DEV_BSIZE * src/storage/storage_backend.c: Remove DEV_BSIZE
+ if (S_ISREG(sb.st_mode)) { +#ifndef __MINGW32__ + info->physical = (unsigned long long)sb.st_blocks * + (unsigned long long)DEV_BSIZE;
Is it worth checking sb.st_blksize rather than DEV_BSIZE on XSI systems where that is part of struct stat? In particular, POSIX allows the block size to be file-system dependent, and some file systems (like NTFS) have 4k rather than 512 as the block size (is anyone daring enough to use NTFS for raw file storage?).
This code is essentially identical to code in storage_backend.c & in the future I will unify it. We did originally use sb.st_blksize in the storage_backend.c impl, but this was not actually correct because it gives the preferred size for efficient I/O, which is not the same as the filesystem block size :-( Hence we have to hardcode 512. The commit was this one, sadly no details in the commit message, but they'll be in the mail archive somewhere: commit e807e4d9e9a766bd00a89b0a9c179edfad52773c Author: Cole Robinson <crobinso@redhat.com> Date: Fri Apr 3 14:13:02 2009 +0000 Fix sparse volume allocation reporting. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Wed, Apr 28, 2010 at 11:58:15AM +0100, Daniel P. Berrange wrote:
On Tue, Apr 27, 2010 at 04:43:08PM -0600, Eric Blake wrote:
On 04/27/2010 01:35 PM, Daniel P. Berrange wrote:
* src/qemu/qemu_driver.c: Implementation of virDomainGetBlockInfo * src/util/storage_file.h: Add DEV_BSIZE * src/storage/storage_backend.c: Remove DEV_BSIZE
+ if (S_ISREG(sb.st_mode)) { +#ifndef __MINGW32__ + info->physical = (unsigned long long)sb.st_blocks * + (unsigned long long)DEV_BSIZE;
Is it worth checking sb.st_blksize rather than DEV_BSIZE on XSI systems where that is part of struct stat? In particular, POSIX allows the block size to be file-system dependent, and some file systems (like NTFS) have 4k rather than 512 as the block size (is anyone daring enough to use NTFS for raw file storage?).
This code is essentially identical to code in storage_backend.c & in the future I will unify it. We did originally use sb.st_blksize in the storage_backend.c impl, but this was not actually correct because it gives the preferred size for efficient I/O, which is not the same as the filesystem block size :-( Hence we have to hardcode 512.
Yeah I remember the issue, it was rather painful trying to do things the Right Way actually led to a bug, fairly nasty.
The commit was this one, sadly no details in the commit message, but they'll be in the mail archive somewhere:
commit e807e4d9e9a766bd00a89b0a9c179edfad52773c Author: Cole Robinson <crobinso@redhat.com> Date: Fri Apr 3 14:13:02 2009 +0000
Fix sparse volume allocation reporting.
Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

virsh # domblkinfo demoguest /dev/hda2 Capacity: 1048576000 Allocation: 104857600 Physical: 104857600 * tools/virsh.c: Implement domblkinfo command mapping to the new virDomainGetBlockInfo API --- tools/virsh.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 48 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5bcf0ed..379eb9d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1045,6 +1045,53 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) } /* + * "domblkinfo" command + */ +static const vshCmdInfo info_domblkinfo[] = { + {"help", N_("domain information")}, + {"desc", N_("Returns basic information about the domain.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domblkinfo[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainBlockInfo info; + virDomainPtr dom; + int ret = TRUE; + const char *device; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return FALSE; + + if (!(device = vshCommandOptString (cmd, "device", NULL))) { + virDomainFree(dom); + return FALSE; + } + + if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) { + virDomainFree(dom); + return FALSE; + } + + vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info.capacity); + vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), info.allocation); + vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info.physical); + + virDomainFree(dom); + return ret; +} + +/* * "suspend" command */ static const vshCmdInfo info_suspend[] = { @@ -8664,6 +8711,7 @@ static const vshCmdDef commands[] = { {"domblkstat", cmdDomblkstat, opts_domblkstat, info_domblkstat}, {"domifstat", cmdDomIfstat, opts_domifstat, info_domifstat}, {"dommemstat", cmdDomMemStat, opts_dommemstat, info_dommemstat}, + {"domblkinfo", cmdDomblkinfo, opts_domblkinfo, info_domblkinfo}, {"domxml-from-native", cmdDomXMLFromNative, opts_domxmlfromnative, info_domxmlfromnative}, {"domxml-to-native", cmdDomXMLToNative, opts_domxmltonative, info_domxmltonative}, {"dumpxml", cmdDumpXML, opts_dumpxml, info_dumpxml}, -- 1.6.6.1

On 04/27/2010 01:35 PM, Daniel P. Berrange wrote:
virsh # domblkinfo demoguest /dev/hda2 Capacity: 1048576000 Allocation: 104857600 Physical: 104857600
* tools/virsh.c: Implement domblkinfo command mapping to the new virDomainGetBlockInfo API
And the changes to tools/virsh.pod?
/* + * "domblkinfo" command + */ +static const vshCmdInfo info_domblkinfo[] = { + {"help", N_("domain information")}, + {"desc", N_("Returns basic information about the domain.")},
Seems like too much copy and paste from dominfo. Maybe: Returns information about a block device used by the domain.
+static const vshCmdOptDef opts_domblkinfo[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")}, + {NULL, 0, 0, NULL} +};
Is there an easy way to query all of the block devices currently tied to a domain, so that it is possible to script a loop that calls domblkinfo for each of those devices? Or would we need to add a new command, maybe 'domblkinfo uuid --all', in addition to your implementation of 'domblkinfo uuid blk'. I think there's enough findings here to request seeing v2 of the patch before giving ack. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Tue, Apr 27, 2010 at 05:00:22PM -0600, Eric Blake wrote:
On 04/27/2010 01:35 PM, Daniel P. Berrange wrote:
virsh # domblkinfo demoguest /dev/hda2 Capacity: 1048576000 Allocation: 104857600 Physical: 104857600
* tools/virsh.c: Implement domblkinfo command mapping to the new virDomainGetBlockInfo API
And the changes to tools/virsh.pod?
/* + * "domblkinfo" command + */ +static const vshCmdInfo info_domblkinfo[] = { + {"help", N_("domain information")}, + {"desc", N_("Returns basic information about the domain.")},
Seems like too much copy and paste from dominfo. Maybe:
Returns information about a block device used by the domain.
Yeah, I was rather fast & loose with this patch :-)
+static const vshCmdOptDef opts_domblkinfo[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")}, + {NULL, 0, 0, NULL} +};
Is there an easy way to query all of the block devices currently tied to a domain, so that it is possible to script a loop that calls domblkinfo for each of those devices? Or would we need to add a new command, maybe 'domblkinfo uuid --all', in addition to your implementation of 'domblkinfo uuid blk'.
To query all block devices, you want to do an xpath query on the guest XML & then iterate on them. This is probably something worth doing, but we can do it as a followup to apply to domblkstats and domifstats at the same time. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Apr 27, 2010 at 08:34:57PM +0100, Daniel P. Berrange wrote:
This is somewhat late for the 0.8.1 release, but this is a rather critical API for some of the use cases of RHEV. It introduces a new API against a virDomainPtr to allow the direct querying of the size of guest block devices. This is modelled on the virStorageVol API for getting size, so I'm pretty confident on the design and impl here.
ACK to the series, late in the process but better now than later :-\ This just misses the python bindings, which can probably be modelled after virStorageVol one too, just one more field ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake