[libvirt] [PATCH 0/5 v4] Atomic API to list network filters
v3 - v4: * Just rebase on the top, and split the patches from v3's large set. Osier Yang (5): list: Define new API virConnectListAllNWFilters list: Implement RPC calls for virConnectListAllNWFilters list: Implement listAllNWFilters list: Expose virConnectListAllNWFilters to Python binding list: Use virConnectListAllNWFilters in virsh daemon/remote.c | 54 +++ include/libvirt/libvirt.h.in |4 +- python/generator.py |1 + python/libvirt-override-api.xml |6 + python/libvirt-override-virConnect.py | 12 +++ python/libvirt-override.c | 48 ++ src/driver.h |5 + src/libvirt.c | 50 ++ src/libvirt_public.syms |1 + src/nwfilter/nwfilter_driver.c| 56 +++ src/remote/remote_driver.c| 63 + src/remote/remote_protocol.x | 13 +++- src/remote_protocol-structs | 12 +++ tools/virsh-nwfilter.c| 163 +++-- 14 files changed, 457 insertions(+), 31 deletions(-) -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] list: Implement RPC calls for virConnectListAllNWFilters
The RPC generator doesn't support returning list of object yet, this patch do the work manually. * daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllNWFilters. * src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllNWFilters. * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 src/remote/remote_driver.c | 63 ++ src/remote/remote_protocol.x | 13 - src/remote_protocol-structs | 12 4 files changed, 141 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d7cce78..00ce9c8 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4373,6 +4373,60 @@ cleanup: return rv; } +static int +remoteDispatchConnectListAllNWFilters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_list_all_nwfilters_args *args, + remote_connect_list_all_nwfilters_ret *ret) +{ +virNWFilterPtr *filters = NULL; +int nfilters = 0; +int i; +int rv = -1; +struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if ((nfilters = virConnectListAllNWFilters(priv-conn, + args-need_results ? filters : NULL, + args-flags)) 0) +goto cleanup; + +if (filters nfilters) { +if (VIR_ALLOC_N(ret-filters.filters_val, nfilters) 0) { +virReportOOMError(); +goto cleanup; +} + +ret-filters.filters_len = nfilters; + +for (i = 0; i nfilters; i++) +make_nonnull_nwfilter(ret-filters.filters_val + i, filters[i]); +} else { +ret-filters.filters_len = 0; +ret-filters.filters_val = NULL; +} + +ret-ret = nfilters; + +rv = 0; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (filters) { +for (i = 0; i nfilters; i++) +virNWFilterFree(filters[i]); +VIR_FREE(filters); +} +return rv; +} + /*- Helpers. -*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b1671ae..2afe5b0 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2907,6 +2907,68 @@ done: return rv; } +static int +remoteConnectListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags) +{ +int rv = -1; +int i; +virNWFilterPtr *tmp_filters = NULL; +remote_connect_list_all_nwfilters_args args; +remote_connect_list_all_nwfilters_ret ret; + +struct private_data *priv = conn-privateData; + +remoteDriverLock(priv); + +args.need_results = !!filters; +args.flags = flags; + +memset(ret, 0, sizeof(ret)); +if (call(conn, + priv, + 0, + REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS, + (xdrproc_t) xdr_remote_connect_list_all_nwfilters_args, + (char *) args, + (xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, + (char *) ret) == -1) +goto done; + +if (filters) { +if (VIR_ALLOC_N(tmp_filters, ret.filters.filters_len + 1) 0) { +virReportOOMError(); +goto cleanup; +} + +for (i = 0; i ret.filters.filters_len; i++) { +tmp_filters[i] = get_nonnull_nwfilter (conn, ret.filters.filters_val[i]); +if (!tmp_filters[i]) { +virReportOOMError(); +goto cleanup; +} +} +*filters = tmp_filters; +tmp_filters = NULL; +} + +rv = ret.ret; + +cleanup: +if (tmp_filters) { +for (i = 0; i ret.filters.filters_len; i++) +if (tmp_filters[i]) +virNWFilterFree(tmp_filters[i]); +VIR_FREE(tmp_filters); +} + +xdr_free((xdrproc_t) xdr_remote_connect_list_all_nwfilters_ret, (char *) ret); + +done: +remoteDriverUnlock(priv); +return rv; +} /*--*/ @@ -6012,6 +6074,7 @@ static virNWFilterDriver nwfilter_driver = { .undefine = remoteNWFilterUndefine, /* 0.8.0 */ .numOfNWFilters =
[libvirt] [PATCH 4/5] list: Expose virConnectListAllNWFilters to Python binding
The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: * Implementation for listAllNWFilters. python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml |6 python/libvirt-override-virConnect.py | 12 python/libvirt-override.c | 48 + 3 files changed, 66 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index a3d6bbb..b8ab823 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -396,6 +396,12 @@ arg name='conn' type='virConnectPtr' info='virConnect connection'/ return type='str *' info='the list of network filter IDs or None in case of error'/ /function +function name='virConnectListAllNWFilters' file='python' + inforeturns list of all network fitlers/info + arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/ + arg name='flags' type='unsigned int' info='optional flags'/ + return type='nwfilter *' info='the list of network filters or None in case of error'/ +/function function name='virNWFilterLookupByUUID' file='python' infoTry to lookup a network filter on the given hypervisor based on its UUID./info return type='virNWFilterPtr' info='a new network filter object or NULL in case of failure'/ diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 0859c36..caca982 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -254,3 +254,15 @@ retlist.append(virNodeDevice(self, _obj=devptr)) return retlist + +def listAllNWFilters(self, flags): +Returns a list of network filter objects +ret = libvirtmod.virConnectListAllNWFilters(self._o, flags) +if ret is None: +raise libvirtError(virConnectListAllNWFilters() failed, conn=self) + +retlist = list() +for filter_ptr in ret: +retlist.append(virNWFilter(self, _obj=filter_ptr)) + +return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index a0478cd..b344ff5 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3764,6 +3764,53 @@ libvirt_virConnectListNWFilters(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virConnectListAllNWFilters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ +PyObject *pyobj_conn; +PyObject *py_retval = NULL; +PyObject *tmp = NULL; +virConnectPtr conn; +virNWFilterPtr *filters = NULL; +int c_retval = 0; +int i; +unsigned int flags; + +if (!PyArg_ParseTuple(args, (char *)Oi:virConnectListAllNWFilters, + pyobj_conn, flags)) +return NULL; +conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_retval = virConnectListAllNWFilters(conn, filters, flags); +LIBVIRT_END_ALLOW_THREADS; +if (c_retval 0) +return VIR_PY_NONE; + +if (!(py_retval = PyList_New(c_retval))) +goto cleanup; + +for (i = 0; i c_retval; i++) { +if (!(tmp = libvirt_virNWFilterPtrWrap(filters[i])) || +PyList_SetItem(py_retval, i, tmp) 0) { +Py_XDECREF(tmp); +Py_DECREF(py_retval); +py_retval = NULL; +goto cleanup; +} +/* python steals the pointer */ +filters[i] = NULL; +} + +cleanup: +for (i = 0; i c_retval; i++) +if (filters[i]) +virNWFilterFree(filters[i]); +VIR_FREE(filters); +return py_retval; +} + +static PyObject * libvirt_virConnectListInterfaces(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; @@ -6142,6 +6189,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) virNWFilterGetUUIDString, libvirt_virNWFilterGetUUIDString, METH_VARARGS, NULL}, {(char *) virNWFilterLookupByUUID, libvirt_virNWFilterLookupByUUID, METH_VARARGS, NULL}, {(char *) virConnectListNWFilters, libvirt_virConnectListNWFilters, METH_VARARGS, NULL}, +{(char *) virConnectListAllNWFilters, libvirt_virConnectListAllNWFilters, METH_VARARGS, NULL}, {(char *) virConnectListInterfaces, libvirt_virConnectListInterfaces, METH_VARARGS, NULL}, {(char *) virConnectListDefinedInterfaces, libvirt_virConnectListDefinedInterfaces, METH_VARARGS, NULL}, {(char *) virConnectListAllInterfaces, libvirt_virConnectListAllInterfaces, METH_VARARGS, NULL}, -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] list: Implement listAllNWFilters
Simply returns the object list. No filtering. src/nwfilter/nwfilter_driver.c: Implement listAllNWFilters --- src/nwfilter/nwfilter_driver.c | 56 1 files changed, 56 insertions(+), 0 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index a30026e..0cf10b8 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -492,6 +492,61 @@ nwfilterListNWFilters(virConnectPtr conn, } +static int +nwfilterListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags) { +virNWFilterDriverStatePtr driver = conn-nwfilterPrivateData; +virNWFilterPtr *tmp_filters = NULL; +int nfilters = 0; +virNWFilterPtr filter = NULL; +virNWFilterObjPtr obj = NULL; +int i; +int ret = -1; + +virCheckFlags(0, -1); + +nwfilterDriverLock(driver); + +if (!filters) { +ret = driver-nwfilters.count; +goto cleanup; +} + +if (VIR_ALLOC_N(tmp_filters, driver-nwfilters.count + 1) 0) { +virReportOOMError(); +goto cleanup; +} + +for (i = 0 ; i driver-nwfilters.count; i++) { +obj = driver-nwfilters.objs[i]; +virNWFilterObjLock(obj); +if (!(filter = virGetNWFilter(conn, obj-def-name, + obj-def-uuid))) { +virNWFilterObjUnlock(obj); +goto cleanup; +} +virNWFilterObjUnlock(obj); +tmp_filters[nfilters++] = filter; +} + +*filters = tmp_filters; +tmp_filters = NULL; +ret = nfilters; + + cleanup: +nwfilterDriverUnlock(driver); +if (tmp_filters) { +for (i = 0; i nfilters; i ++) { +if (tmp_filters[i]) +virNWFilterFree(tmp_filters[i]); +} +} +VIR_FREE(tmp_filters); + +return ret; +} + static virNWFilterPtr nwfilterDefine(virConnectPtr conn, const char *xml) @@ -627,6 +682,7 @@ static virNWFilterDriver nwfilterDriver = { .close = nwfilterClose, /* 0.8.0 */ .numOfNWFilters = nwfilterNumNWFilters, /* 0.8.0 */ .listNWFilters = nwfilterListNWFilters, /* 0.8.0 */ +.listAllNWFilters = nwfilterListAllNWFilters, /* 0.10.2 */ .nwfilterLookupByName = nwfilterLookupByName, /* 0.8.0 */ .nwfilterLookupByUUID = nwfilterLookupByUUID, /* 0.8.0 */ .defineXML = nwfilterDefine, /* 0.8.0 */ -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] list: Define new API virConnectListAllNWFilters
This is to list the network fitler objects. No flags are supported include/libvirt/libvirt.h.in: Declare enum virConnectListAllNWFilterFlags and virConnectListAllNWFilters. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllNWFilters) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in |4 ++- python/generator.py |1 + src/driver.h |5 src/libvirt.c| 50 ++ src/libvirt_public.syms |1 + 5 files changed, 60 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 96d0760..86f640d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4115,7 +4115,9 @@ int virConnectNumOfNWFilters (virConnectPtr conn); int virConnectListNWFilters (virConnectPtr conn, char **const names, int maxnames); - +int virConnectListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags); /* * Lookup nwfilter by name or uuid */ diff --git a/python/generator.py b/python/generator.py index a8e4ec6..d3163e4 100755 --- a/python/generator.py +++ b/python/generator.py @@ -465,6 +465,7 @@ skip_function = ( 'virConnectListAllNetworks', # overridden in virConnect.py 'virConnectListAllInterfaces', # overridden in virConnect.py 'virConnectListAllNodeDevices', # overridden in virConnect.py +'virConnectListAllNWFilters', # overridden in virConnect.py 'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index 34a94af..9984a85 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1638,6 +1638,10 @@ typedef int (*virDrvConnectListNWFilters) (virConnectPtr conn, char **const names, int maxnames); +typedef int +(*virDrvConnectListAllNWFilters) (virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags); typedef virNWFilterPtr (*virDrvNWFilterLookupByName) (virConnectPtr conn, const char *name); @@ -1675,6 +1679,7 @@ struct _virNWFilterDriver { virDrvConnectNumOfNWFilters numOfNWFilters; virDrvConnectListNWFilters listNWFilters; +virDrvConnectListAllNWFilters listAllNWFilters; virDrvNWFilterLookupByName nwfilterLookupByName; virDrvNWFilterLookupByUUID nwfilterLookupByUUID; virDrvNWFilterDefineXML defineXML; diff --git a/src/libvirt.c b/src/libvirt.c index b8d0bec..55a2f4e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16153,6 +16153,56 @@ error: return -1; } +/** + * virConnectListAllNWFilters: + * @conn: Pointer to the hypervisor connection. + * @filters: Pointer to a variable to store the array containing the network + * filter objects or NULL if the list is not required (just returns + * number of network filters). + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect the list of network filters, and allocate an array to store those + * objects. + * + * Returns the number of network filters found or -1 and sets @filters to NULL + * in case of error. On success, the array stored into @filters is guaranteed to + * have an extra allocated element set to NULL but not included in the return count, + * to make iteration easier. The caller is responsible for calling + * virNWFilterFree() on each array element, then calling free() on @filters. + */ +int +virConnectListAllNWFilters(virConnectPtr conn, + virNWFilterPtr **filters, + unsigned int flags) +{ +VIR_DEBUG(conn=%p, filters=%p, flags=%x, conn, filters, flags); + +virResetLastError(); + +if (filters) +*filters = NULL; + +if (!VIR_IS_CONNECT(conn)) { +virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} + +if (conn-nwfilterDriver +conn-nwfilterDriver-listAllNWFilters) { +int ret; +ret = conn-nwfilterDriver-listAllNWFilters(conn, filters, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(conn); +return -1; +} /** * virConnectListNWFilters: diff --git a/src/libvirt_public.syms
[libvirt] [PATCH 5/5] list: Use virConnectListAllNWFilters in virsh
tools/virsh-nwfilter.c: * vshNWFilterSorter to sort network filters by name * vshNWFilterListFree to free the network filter objects list. * vshNWFilterListCollect to collect the network filter objects, trying to use new API first, fall back to older APIs if it's not supported. --- tools/virsh-nwfilter.c | 163 +++- 1 files changed, 134 insertions(+), 29 deletions(-) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 5169d38..57cf2b7 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -190,6 +190,134 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; } +static int +vshNWFilterSorter(const void *a, const void *b) +{ +virNWFilterPtr *fa = (virNWFilterPtr *) a; +virNWFilterPtr *fb = (virNWFilterPtr *) b; + +if (*fa !*fb) +return -1; + +if (!*fa) +return *fb != NULL; + +return vshStrcasecmp(virNWFilterGetName(*fa), + virNWFilterGetName(*fb)); +} + +struct vshNWFilterList { +virNWFilterPtr *filters; +size_t nfilters; +}; +typedef struct vshNWFilterList *vshNWFilterListPtr; + +static void +vshNWFilterListFree(vshNWFilterListPtr list) +{ +int i; + +if (list list-nfilters) { +for (i = 0; i list-nfilters; i++) { +if (list-filters[i]) +virNWFilterFree(list-filters[i]); +} +VIR_FREE(list-filters); +} +VIR_FREE(list); +} + +static vshNWFilterListPtr +vshNWFilterListCollect(vshControl *ctl, + unsigned int flags) +{ +vshNWFilterListPtr list = vshMalloc(ctl, sizeof(*list)); +int i; +int ret; +virNWFilterPtr filter; +bool success = false; +size_t deleted = 0; +int nfilters = 0; +char **names = NULL; + +/* try the list with flags support (0.10.2 and later) */ +if ((ret = virConnectListAllNWFilters(ctl-conn, + list-filters, + flags)) = 0) { +list-nfilters = ret; +goto finished; +} + +/* check if the command is actually supported */ +if (last_error last_error-code == VIR_ERR_NO_SUPPORT) { +vshResetLibvirtError(); +goto fallback; +} + +/* there was an error during the call */ +vshError(ctl, %s, _(Failed to list node filters)); +goto cleanup; + + +fallback: +/* fall back to old method (0.9.13 and older) */ +vshResetLibvirtError(); + +nfilters = virConnectNumOfNWFilters(ctl-conn); +if (nfilters 0) { +vshError(ctl, %s, _(Failed to count network filters)); +goto cleanup; +} + +if (nfilters == 0) +return list; + +names = vshMalloc(ctl, sizeof(char *) * nfilters); + +nfilters = virConnectListNWFilters(ctl-conn, names, nfilters); +if (nfilters 0) { +vshError(ctl, %s, _(Failed to list network filters)); +goto cleanup; +} + +list-filters = vshMalloc(ctl, sizeof(virNWFilterPtr) * (nfilters)); +list-nfilters = 0; + +/* get the network filters */ +for (i = 0; i nfilters ; i++) { +if (!(filter = virNWFilterLookupByName(ctl-conn, names[i]))) +continue; +list-filters[list-nfilters++] = filter; +} + +/* truncate network filters that weren't found */ +deleted = nfilters - list-nfilters; + +finished: +/* sort the list */ +if (list-filters list-nfilters) +qsort(list-filters, list-nfilters, + sizeof(*list-filters), vshNWFilterSorter); + +/* truncate the list for not found filter objects */ +if (deleted) +VIR_SHRINK_N(list-filters, list-nfilters, deleted); + +success = true; + +cleanup: +for (i = 0; i nfilters; i++) +VIR_FREE(names[i]); +VIR_FREE(names); + +if (!success) { +vshNWFilterListFree(list); +list = NULL; +} + +return list; +} + /* * nwfilter-list command */ @@ -206,50 +334,27 @@ static const vshCmdOptDef opts_nwfilter_list[] = { static bool cmdNWFilterList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { -int numfilters, i; -char **names; +int i; char uuid[VIR_UUID_STRING_BUFLEN]; +vshNWFilterListPtr list = NULL; -numfilters = virConnectNumOfNWFilters(ctl-conn); -if (numfilters 0) { -vshError(ctl, %s, _(Failed to list network filters)); -return false; -} - -names = vshMalloc(ctl, sizeof(char *) * numfilters); - -if ((numfilters = virConnectListNWFilters(ctl-conn, names, - numfilters)) 0) { -vshError(ctl, %s, _(Failed to list network filters)); -VIR_FREE(names); +if (!(list = vshNWFilterListCollect(ctl, 0))) return false; -} - -qsort(names[0], numfilters, sizeof(char *), vshNameSorter); vshPrintExtra(ctl, %-36s %-20s \n, _(UUID), _(Name)); vshPrintExtra(ctl,
[libvirt] [PATCH 0/6 v4] Atomic API to list secrets
v3 - v4: * Just rebase on the top, split the patches from v3's larget set Osier Yang (6): list: Define new API virConnectListAllSecrets list: Implement RPC calls for virConnectListAllSecrets list: Implement listAllSecrets list: Expose virConnectListAllSecrets to Python binding list: Expose virConnectListAllSecrets to Python binding virsh: Remove unused vshNameSorter daemon/remote.c | 54 ++ include/libvirt/libvirt.h.in |3 + python/generator.py |1 + python/libvirt-override-api.xml |6 + python/libvirt-override-virConnect.py | 12 ++ python/libvirt-override.c | 48 + src/driver.h |5 + src/libvirt.c | 50 + src/libvirt_public.syms |1 + src/remote/remote_driver.c| 64 src/remote/remote_protocol.x | 13 ++- src/remote_protocol-structs | 12 ++ src/secret/secret_driver.c| 59 +++- tools/virsh-secret.c | 182 +++- tools/virsh.c |9 -- tools/virsh.h |1 - 16 files changed, 479 insertions(+), 41 deletions(-) -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] list: Define new API virConnectListAllSecrets
This is to list the secret objects. No flags are supported include/libvirt/libvirt.h.in: Declare enum virConnectListAllSecretFlags and virConnectListAllSecrets. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllSecrets) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in |3 ++ python/generator.py |1 + src/driver.h |5 src/libvirt.c| 50 ++ src/libvirt_public.syms |1 + 5 files changed, 60 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 86f640d..75257d9 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3238,6 +3238,9 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); +int virConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags); virSecretPtrvirSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virSecretPtrvirSecretLookupByUUIDString(virConnectPtr conn, diff --git a/python/generator.py b/python/generator.py index d3163e4..955c893 100755 --- a/python/generator.py +++ b/python/generator.py @@ -466,6 +466,7 @@ skip_function = ( 'virConnectListAllInterfaces', # overridden in virConnect.py 'virConnectListAllNodeDevices', # overridden in virConnect.py 'virConnectListAllNWFilters', # overridden in virConnect.py +'virConnectListAllSecrets', # overridden in virConnect.py 'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index 9984a85..3e69dae 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1567,6 +1567,10 @@ typedef int (*virDrvListSecrets) (virConnectPtr conn, char **uuids, int maxuuids); +typedef int +(*virDrvListAllSecrets)(virConnectPtr conn, +virSecretPtr **secrets, +unsigned int flags); typedef struct _virSecretDriver virSecretDriver; typedef virSecretDriver *virSecretDriverPtr; @@ -1588,6 +1592,7 @@ struct _virSecretDriver { virDrvNumOfSecrets numOfSecrets; virDrvListSecrets listSecrets; +virDrvListAllSecretslistAllSecrets; virDrvSecretLookupByUUIDlookupByUUID; virDrvSecretLookupByUsage lookupByUsage; virDrvSecretDefineXML defineXML; diff --git a/src/libvirt.c b/src/libvirt.c index 55a2f4e..f6eb6b9 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -14582,6 +14582,56 @@ error: } /** + * virConnectListAllSecrets: + * @conn: Pointer to the hypervisor connection. + * @secrets: Pointer to a variable to store the array containing the secret + * objects or NULL if the list is not required (just returns the + * number of secrets). + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect the list of secrets, and allocate an array to store those objects. + * + * Returns the number of secrets found or -1 and sets @secrets to NULL in case + * of error. On success, the array stored into @secrets is guaranteed to + * have an extra allocated element set to NULL but not included in the return count, + * to make iteration easier. The caller is responsible for calling + * virSecretFree() on each array element, then calling free() on @secrets. + */ +int +virConnectListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) +{ +VIR_DEBUG(conn=%p, secrets=%p, flags=%x, conn, secrets, flags); + +virResetLastError(); + +if (secrets) +*secrets = NULL; + +if (!VIR_IS_CONNECT(conn)) { +virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} + +if (conn-secretDriver +conn-secretDriver-listAllSecrets) { +int ret; +ret = conn-secretDriver-listAllSecrets(conn, secrets, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(conn); +return -1; +} + +/** * virConnectListSecrets: * @conn: virConnect connection * @uuids: Pointer to an array to store the UUIDs diff --git
[libvirt] [PATCH 4/6] list: Expose virConnectListAllSecrets to Python binding
The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: Implementation for listAllSecrets. python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml |6 python/libvirt-override-virConnect.py | 12 python/libvirt-override.c | 48 + 3 files changed, 66 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index b8ab823..4f609ee 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -368,6 +368,12 @@ arg name='conn' type='virConnectPtr' info='virConnect connection'/ return type='str *' info='the list of secret IDs or None in case of error'/ /function +function name='virConnectListAllSecrets' file='python' + inforeturns list of all interfaces/info + arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/ + arg name='flags' type='unsigned int' info='optional flags'/ + return type='secret *' info='the list of secrets or None in case of error'/ +/function function name='virSecretSetValue' file='libvirt' module='libvirt' infoAssociates a value with a secret./info return type='int' info='0 on success, -1 on failure.'/ diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index caca982..6bec66d 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -266,3 +266,15 @@ retlist.append(virNWFilter(self, _obj=filter_ptr)) return retlist + +def listAllSecrets(self, flags): +Returns a list of secret objects +ret = libvirtmod.virConnectListAllSecrets(self._o, flags) +if ret is None: +raise libvirtError(virConnectListAllSecrets() failed, conn=self) + +retlist = list() +for secret_ptr in ret: +retlist.append(virSecret(self, _obj=secret_ptr)) + +return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index b344ff5..1fab8ec 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3592,6 +3592,53 @@ libvirt_virConnectListSecrets(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virConnectListAllSecrets(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ +PyObject *pyobj_conn; +PyObject *py_retval = NULL; +PyObject *tmp = NULL; +virConnectPtr conn; +virSecretPtr *secrets = NULL; +int c_retval = 0; +int i; +unsigned int flags; + +if (!PyArg_ParseTuple(args, (char *)Oi:virConnectListAllSecrets, + pyobj_conn, flags)) +return NULL; +conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_retval = virConnectListAllSecrets(conn, secrets, flags); +LIBVIRT_END_ALLOW_THREADS; +if (c_retval 0) +return VIR_PY_NONE; + +if (!(py_retval = PyList_New(c_retval))) +goto cleanup; + +for (i = 0; i c_retval; i++) { +if (!(tmp = libvirt_virSecretPtrWrap(secrets[i])) || +PyList_SetItem(py_retval, i, tmp) 0) { +Py_XDECREF(tmp); +Py_DECREF(py_retval); +py_retval = NULL; +goto cleanup; +} +/* python steals the pointer */ +secrets[i] = NULL; +} + +cleanup: +for (i = 0; i c_retval; i++) +if (secrets[i]) +virSecretFree(secrets[i]); +VIR_FREE(secrets); +return py_retval; +} + +static PyObject * libvirt_virSecretGetValue(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; @@ -6183,6 +6230,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) virSecretGetUUIDString, libvirt_virSecretGetUUIDString, METH_VARARGS, NULL}, {(char *) virSecretLookupByUUID, libvirt_virSecretLookupByUUID, METH_VARARGS, NULL}, {(char *) virConnectListSecrets, libvirt_virConnectListSecrets, METH_VARARGS, NULL}, +{(char *) virConnectListAllSecrets, libvirt_virConnectListAllSecrets, METH_VARARGS, NULL}, {(char *) virSecretGetValue, libvirt_virSecretGetValue, METH_VARARGS, NULL}, {(char *) virSecretSetValue, libvirt_virSecretSetValue, METH_VARARGS, NULL}, {(char *) virNWFilterGetUUID, libvirt_virNWFilterGetUUID, METH_VARARGS, NULL}, -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] list: Implement listAllSecrets
Simply returns the object list. No filtering. src/secret/secret_driver.c: Implement listAllSecrets --- src/secret/secret_driver.c | 59 +++- 1 files changed, 58 insertions(+), 1 deletions(-) diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 7f92776..ed759ed 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -601,7 +601,6 @@ cleanup: return -1; } - static const char * secretUsageIDForDef(virSecretDefPtr def) { @@ -620,6 +619,63 @@ secretUsageIDForDef(virSecretDefPtr def) } } +static int +secretListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) { +virSecretDriverStatePtr driver = conn-secretPrivateData; +virSecretPtr *tmp_secrets = NULL; +int nsecrets = 0; +int ret_nsecrets = 0; +virSecretPtr secret = NULL; +virSecretEntryPtr entry = NULL; +int i = 0; +int ret = -1; + +virCheckFlags(0, -1); + +secretDriverLock(driver); + +for (entry = driver-secrets; entry != NULL; entry = entry-next) +nsecrets++; + +if (!secrets) { +ret = nsecrets; +goto cleanup; +} + +if (VIR_ALLOC_N(tmp_secrets, nsecrets + 1) 0) { +virReportOOMError(); +goto cleanup; +} + +for (entry = driver-secrets; entry != NULL; entry = entry-next) { +if (!(secret = virGetSecret(conn, +entry-def-uuid, +entry-def-usage_type, +secretUsageIDForDef(entry-def +goto cleanup; +tmp_secrets[ret_nsecrets++] = secret; +} + +*secrets = tmp_secrets; +tmp_secrets = NULL; +ret = ret_nsecrets; + + cleanup: +secretDriverUnlock(driver); +if (tmp_secrets) { +for (i = 0; i ret_nsecrets; i ++) { +if (tmp_secrets[i]) +virSecretFree(tmp_secrets[i]); +} +} +VIR_FREE(tmp_secrets); + +return ret; +} + + static virSecretPtr secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { @@ -1072,6 +1128,7 @@ static virSecretDriver secretDriver = { .close = secretClose, /* 0.7.1 */ .numOfSecrets = secretNumOfSecrets, /* 0.7.1 */ .listSecrets = secretListSecrets, /* 0.7.1 */ +.listAllSecrets = secretListAllSecrets, /* 0.10.2 */ .lookupByUUID = secretLookupByUUID, /* 0.7.1 */ .lookupByUsage = secretLookupByUsage, /* 0.7.1 */ .defineXML = secretDefineXML, /* 0.7.1 */ -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] list: Implement RPC calls for virConnectListAllSecrets
The RPC generator doesn't support returning list of object yet, this patch do the work manually. * daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllSecrets. * src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllSecrets. * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_SECRETS and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 +++ src/remote/remote_driver.c | 64 ++ src/remote/remote_protocol.x | 13 - src/remote_protocol-structs | 12 4 files changed, 142 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 00ce9c8..6e2bd8d 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4427,6 +4427,60 @@ cleanup: return rv; } +static int +remoteDispatchConnectListAllSecrets(virNetServerPtr server ATTRIBUTE_UNUSED, +virNetServerClientPtr client, +virNetMessagePtr msg ATTRIBUTE_UNUSED, +virNetMessageErrorPtr rerr, +remote_connect_list_all_secrets_args *args, +remote_connect_list_all_secrets_ret *ret) +{ +virSecretPtr *secrets = NULL; +int nsecrets = 0; +int i; +int rv = -1; +struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if ((nsecrets = virConnectListAllSecrets(priv-conn, + args-need_results ? secrets : NULL, + args-flags)) 0) +goto cleanup; + +if (secrets nsecrets) { +if (VIR_ALLOC_N(ret-secrets.secrets_val, nsecrets) 0) { +virReportOOMError(); +goto cleanup; +} + +ret-secrets.secrets_len = nsecrets; + +for (i = 0; i nsecrets; i++) +make_nonnull_secret(ret-secrets.secrets_val + i, secrets[i]); +} else { +ret-secrets.secrets_len = 0; +ret-secrets.secrets_val = NULL; +} + +ret-ret = nsecrets; + +rv = 0; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (secrets) { +for (i = 0; i nsecrets; i++) +virSecretFree(secrets[i]); +VIR_FREE(secrets); +} +return rv; +} + /*- Helpers. -*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 2afe5b0..0a92339 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2970,6 +2970,69 @@ done: return rv; } +static int +remoteConnectListAllSecrets(virConnectPtr conn, +virSecretPtr **secrets, +unsigned int flags) +{ +int rv = -1; +int i; +virSecretPtr *tmp_secrets = NULL; +remote_connect_list_all_secrets_args args; +remote_connect_list_all_secrets_ret ret; + +struct private_data *priv = conn-privateData; + +remoteDriverLock(priv); + +args.need_results = !!secrets; +args.flags = flags; + +memset(ret, 0, sizeof(ret)); +if (call(conn, + priv, + 0, + REMOTE_PROC_CONNECT_LIST_ALL_SECRETS, + (xdrproc_t) xdr_remote_connect_list_all_secrets_args, + (char *) args, + (xdrproc_t) xdr_remote_connect_list_all_secrets_ret, + (char *) ret) == -1) +goto done; + +if (secrets) { +if (VIR_ALLOC_N(tmp_secrets, ret.secrets.secrets_len + 1) 0) { +virReportOOMError(); +goto cleanup; +} + +for (i = 0; i ret.secrets.secrets_len; i++) { +tmp_secrets[i] = get_nonnull_secret (conn, ret.secrets.secrets_val[i]); +if (!tmp_secrets[i]) { +virReportOOMError(); +goto cleanup; +} +} +*secrets = tmp_secrets; +tmp_secrets = NULL; +} + +rv = ret.ret; + +cleanup: +if (tmp_secrets) { +for (i = 0; i ret.secrets.secrets_len; i++) +if (tmp_secrets[i]) +virSecretFree(tmp_secrets[i]); +VIR_FREE(tmp_secrets); +} + +xdr_free((xdrproc_t) xdr_remote_connect_list_all_secrets_ret, (char *) ret); + +done: +remoteDriverUnlock(priv); +return rv; +} + /*--*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -6038,6 +6101,7 @@ static virSecretDriver secret_driver = { .close = remoteSecretClose, /* 0.7.1 */ .numOfSecrets = remoteNumOfSecrets, /* 0.7.1 */ .listSecrets =
[libvirt] [PATCH 6/6] virsh: Remove unused vshNameSorter
vshNameSorter now is not used anymore. Remove it. --- tools/virsh.c |9 - tools/virsh.h |1 - 2 files changed, 0 insertions(+), 10 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 684f7ca..8e794ea 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -136,15 +136,6 @@ _vshStrdup(vshControl *ctl, const char *s, const char *filename, int line) /* Poison the raw allocating identifiers in favor of our vsh variants. */ #define strdup use_vshStrdup_instead_of_strdup -int -vshNameSorter(const void *a, const void *b) -{ -const char **sa = (const char**)a; -const char **sb = (const char**)b; - -return vshStrcasecmp(*sa, *sb); -} - double vshPrettyCapacity(unsigned long long val, const char **unit) { diff --git a/tools/virsh.h b/tools/virsh.h index 2758d37..ac12b8b 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -314,7 +314,6 @@ void vshDebug(vshControl *ctl, int level, const char *format, ...) /* User visible sort, so we want locale-specific case comparison. */ # define vshStrcasecmp(S1, S2) strcasecmp(S1, S2) -int vshNameSorter(const void *a, const void *b); int vshDomainState(vshControl *ctl, virDomainPtr dom, int *reason); virTypedParameterPtr vshFindTypedParamByName(const char *name, -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] list: Expose virConnectListAllSecrets to Python binding
The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: Implementation for listAllSecrets. python/libvirt-override.c: Implementation for the wrapper. --- tools/virsh-secret.c | 182 ++ 1 files changed, 153 insertions(+), 29 deletions(-) diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index abcfdfe..c6193cc 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -290,6 +290,139 @@ cleanup: return ret; } +static int +vshSecretSorter(const void *a, const void *b) +{ +virSecretPtr *sa = (virSecretPtr *) a; +virSecretPtr *sb = (virSecretPtr *) b; +char uuid_sa[VIR_UUID_STRING_BUFLEN]; +char uuid_sb[VIR_UUID_STRING_BUFLEN]; + +if (*sa !*sb) +return -1; + +if (!*sa) +return *sb != NULL; + +virSecretGetUUIDString(*sa, uuid_sa); +virSecretGetUUIDString(*sb, uuid_sb); + +return vshStrcasecmp(uuid_sa, uuid_sb); +} + +struct vshSecretList { +virSecretPtr *secrets; +size_t nsecrets; +}; +typedef struct vshSecretList *vshSecretListPtr; + +static void +vshSecretListFree(vshSecretListPtr list) +{ +int i; + +if (list list-nsecrets) { +for (i = 0; i list-nsecrets; i++) { +if (list-secrets[i]) +virSecretFree(list-secrets[i]); +} +VIR_FREE(list-secrets); +} +VIR_FREE(list); +} + +static vshSecretListPtr +vshSecretListCollect(vshControl *ctl, + unsigned int flags) +{ +vshSecretListPtr list = vshMalloc(ctl, sizeof(*list)); +int i; +int ret; +virSecretPtr secret; +bool success = false; +size_t deleted = 0; +int nsecrets = 0; +char **uuids = NULL; + +/* try the list with flags support (0.10.2 and later) */ +if ((ret = virConnectListAllSecrets(ctl-conn, +list-secrets, +flags)) = 0) { +list-nsecrets = ret; +goto finished; +} + +/* check if the command is actually supported */ +if (last_error last_error-code == VIR_ERR_NO_SUPPORT) { +virFreeError(last_error); +last_error = NULL; +goto fallback; +} + +/* there was an error during the call */ +vshError(ctl, %s, _(Failed to list node secrets)); +goto cleanup; + + +fallback: +/* fall back to old method (0.9.13 and older) */ +virResetLastError(); + +nsecrets = virConnectNumOfSecrets(ctl-conn); +if (nsecrets 0) { +vshError(ctl, %s, _(Failed to count secrets)); +goto cleanup; +} + +if (nsecrets == 0) +return list; + +uuids = vshMalloc(ctl, sizeof(char *) * nsecrets); + +nsecrets = virConnectListSecrets(ctl-conn, uuids, nsecrets); +if (nsecrets 0) { +vshError(ctl, %s, _(Failed to list secrets)); +goto cleanup; +} + +list-secrets = vshMalloc(ctl, sizeof(virSecretPtr) * (nsecrets)); +list-nsecrets = 0; + +/* get the secrets */ +for (i = 0; i nsecrets ; i++) { +if (!(secret = virSecretLookupByUUIDString(ctl-conn, uuids[i]))) +continue; +list-secrets[list-nsecrets++] = secret; +} + +/* truncate secrets that weren't found */ +deleted = nsecrets - list-nsecrets; + +finished: +/* sort the list */ +if (list-secrets list-nsecrets) +qsort(list-secrets, list-nsecrets, + sizeof(*list-secrets), vshSecretSorter); + +/* truncate the list for not found secret objects */ +if (deleted) +VIR_SHRINK_N(list-secrets, list-nsecrets, deleted); + +success = true; + +cleanup: +for (i = 0; i nsecrets; i++) +VIR_FREE(uuids[i]); +VIR_FREE(uuids); + +if (!success) { +vshSecretListFree(list); +list = NULL; +} + +return list; +} + /* * secret-list command */ @@ -302,56 +435,47 @@ static const vshCmdInfo info_secret_list[] = { static bool cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { -int maxuuids = 0, i; -char **uuids = NULL; - -maxuuids = virConnectNumOfSecrets(ctl-conn); -if (maxuuids 0) { -vshError(ctl, %s, _(Failed to list secrets)); -return false; -} -uuids = vshMalloc(ctl, sizeof(*uuids) * maxuuids); +int i; +vshSecretListPtr list = NULL; +bool ret = false; -maxuuids = virConnectListSecrets(ctl-conn, uuids, maxuuids); -if (maxuuids 0) { -vshError(ctl, %s, _(Failed to list secrets)); -VIR_FREE(uuids); +if (!(list = vshSecretListCollect(ctl, 0))) return false; -} - -qsort(uuids, maxuuids, sizeof(char *), vshNameSorter); vshPrintExtra(ctl, %-36s %s\n, _(UUID), _(Usage)); vshPrintExtra(ctl, ---\n); -for (i = 0; i maxuuids; i++) { -
[libvirt] [PATCH 09/10 v5] list: Use virConnectListAllStoragePools in virsh
tools/virsh-pool.c: * vshStoragePoolSorter to sort the pool list by pool name. * struct vshStoragePoolList to present the pool list, pool info is collected by list-poolinfo if 'details' is specified by user. * vshStoragePoolListFree to free the pool list * vshStoragePoolListCollect to collect the pool list, new API virStorageListAllPools is tried first, if it's not supported, fall back to older APIs. * New options --persistent, --transient, --autostart, --no-autostart and --type for pool-list. --persistent or --transient is to filter the returned pool list by whether the pool is persistent or not. --autostart or --no-autostart is to filter the returned pool list by whether the pool is autostarting or not. --type is to filter the pools by pool types. E.g. % virsh pool-list --all --persistent --type dir,disk tools/virsh.pod: * Add documentations for the new options. --- tools/virsh-pool.c | 434 tools/virsh.pod| 24 +++- 2 files changed, 358 insertions(+), 100 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index fd239d2..365d035 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -36,6 +36,7 @@ #include memory.h #include util.h #include xml.h +#include conf/storage_conf.h virStoragePoolPtr vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, @@ -551,6 +552,232 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; } +static int +vshStoragePoolSorter(const void *a, const void *b) +{ +virStoragePoolPtr *pa = (virStoragePoolPtr *) a; +virStoragePoolPtr *pb = (virStoragePoolPtr *) b; + +if (*pa !*pb) +return -1; + +if (!*pa) +return *pb != NULL; + +return vshStrcasecmp(virStoragePoolGetName(*pa), + virStoragePoolGetName(*pb)); +} + +struct vshStoragePoolList { +virStoragePoolPtr *pools; +size_t npools; +}; +typedef struct vshStoragePoolList *vshStoragePoolListPtr; + +static void +vshStoragePoolListFree(vshStoragePoolListPtr list) +{ +int i; + +if (list list-pools) { +for (i = 0; i list-npools; i++) { +if (list-pools[i]) +virStoragePoolFree(list-pools[i]); +} +VIR_FREE(list-pools); +} +VIR_FREE(list); +} + +static vshStoragePoolListPtr +vshStoragePoolListCollect(vshControl *ctl, + unsigned int flags) +{ +vshStoragePoolListPtr list = vshMalloc(ctl, sizeof(*list)); +int i; +int ret; +char **names = NULL; +virStoragePoolPtr pool; +bool success = false; +size_t deleted = 0; +int persistent; +int autostart; +int nActivePools = 0; +int nInactivePools = 0; +int nAllPools = 0; + +/* try the list with flags support (0.10.0 and later) */ +if ((ret = virConnectListAllStoragePools(ctl-conn, + list-pools, + flags)) = 0) { +list-npools = ret; +goto finished; +} + +/* check if the command is actually supported */ +if (last_error last_error-code == VIR_ERR_NO_SUPPORT) { +vshResetLibvirtError(); +goto fallback; +} + +if (last_error last_error-code == VIR_ERR_INVALID_ARG) { +/* try the new API again but mask non-guaranteed flags */ +unsigned int newflags = flags (VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE); +vshResetLibvirtError(); +if ((ret = virConnectListAllStoragePools(ctl-conn, list-pools, + newflags)) = 0) { +list-npools = ret; +goto filter; +} +} + +/* there was an error during the first or second call */ +vshError(ctl, %s, _(Failed to list pools)); +goto cleanup; + + +fallback: +/* fall back to old method (0.9.13 and older) */ +vshResetLibvirtError(); + +/* There is no way to get the pool type */ +if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE)) { +vshError(ctl, %s, _(Filtering using --type is not supported + by this libvirt)); +goto cleanup; +} + +/* Get the number of active pools */ +if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || +MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE)) { +if ((nActivePools = virConnectNumOfStoragePools(ctl-conn)) 0) { +vshError(ctl, %s, _(Failed to get the number of active pools )); +goto cleanup; +} +} + +/* Get the number of inactive pools */ +if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || +MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE)) { +if ((nInactivePools = virConnectNumOfDefinedStoragePools(ctl-conn)) 0) { +vshError(ctl, %s,
Re: [libvirt] [PATCH 09/10] list: Use virConnectListAllStoragePools in virsh
Please ignore this, and review v5 instead. (I forgot to cleanup the #if 0...#endif) On 2012年09月04日 23:16, Osier Yang wrote: tools/virsh-pool.c: * vshStoragePoolSorter to sort the pool list by pool name. * struct vshStoragePoolList to present the pool list, pool info is collected by list-poolinfo if 'details' is specified by user. * vshStoragePoolListFree to free the pool list * vshStoragePoolListCollect to collect the pool list, new API virStorageListAllPools is tried first, if it's not supported, fall back to older APIs. * New options --persistent, --transient, --autostart, --no-autostart and --type for pool-list. --persistent or --transient is to filter the returned pool list by whether the pool is persistent or not. --autostart or --no-autostart is to filter the returned pool list by whether the pool is autostarting or not. --type is to filter the pools by pool types. E.g. % virsh pool-list --all --persistent --type dir,disk tools/virsh.pod: * Add documentations for the new options. --- tools/virsh-pool.c | 631 +++- tools/virsh.pod| 24 ++- 2 files changed, 652 insertions(+), 3 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index fd239d2..0b328cc 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -36,6 +36,7 @@ #include memory.h #include util.h #include xml.h +#include conf/storage_conf.h virStoragePoolPtr vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, @@ -551,6 +552,232 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; } +static int +vshStoragePoolSorter(const void *a, const void *b) +{ +virStoragePoolPtr *pa = (virStoragePoolPtr *) a; +virStoragePoolPtr *pb = (virStoragePoolPtr *) b; + +if (*pa !*pb) +return -1; + +if (!*pa) +return *pb != NULL; + +return vshStrcasecmp(virStoragePoolGetName(*pa), + virStoragePoolGetName(*pb)); +} + +struct vshStoragePoolList { +virStoragePoolPtr *pools; +size_t npools; +}; +typedef struct vshStoragePoolList *vshStoragePoolListPtr; + +static void +vshStoragePoolListFree(vshStoragePoolListPtr list) +{ +int i; + +if (list list-pools) { +for (i = 0; i list-npools; i++) { +if (list-pools[i]) +virStoragePoolFree(list-pools[i]); +} +VIR_FREE(list-pools); +} +VIR_FREE(list); +} + +static vshStoragePoolListPtr +vshStoragePoolListCollect(vshControl *ctl, + unsigned int flags) +{ +vshStoragePoolListPtr list = vshMalloc(ctl, sizeof(*list)); +int i; +int ret; +char **names = NULL; +virStoragePoolPtr pool; +bool success = false; +size_t deleted = 0; +int persistent; +int autostart; +int nActivePools = 0; +int nInactivePools = 0; +int nAllPools = 0; + +/* try the list with flags support (0.10.0 and later) */ +if ((ret = virConnectListAllStoragePools(ctl-conn, +list-pools, + flags))= 0) { +list-npools = ret; +goto finished; +} + +/* check if the command is actually supported */ +if (last_error last_error-code == VIR_ERR_NO_SUPPORT) { +vshResetLibvirtError(); +goto fallback; +} + +if (last_error last_error-code == VIR_ERR_INVALID_ARG) { +/* try the new API again but mask non-guaranteed flags */ +unsigned int newflags = flags (VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE); +vshResetLibvirtError(); +if ((ret = virConnectListAllStoragePools(ctl-conn,list-pools, + newflags))= 0) { +list-npools = ret; +goto filter; +} +} + +/* there was an error during the first or second call */ +vshError(ctl, %s, _(Failed to list pools)); +goto cleanup; + + +fallback: +/* fall back to old method (0.9.13 and older) */ +vshResetLibvirtError(); + +/* There is no way to get the pool type */ +if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE)) { +vshError(ctl, %s, _(Filtering using --type is not supported + by this libvirt)); +goto cleanup; +} + +/* Get the number of active pools */ +if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || +MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE)) { +if ((nActivePools = virConnectNumOfStoragePools(ctl-conn)) 0) { +vshError(ctl, %s, _(Failed to get the number of active pools )); +goto cleanup; +} +} + +/* Get the number of inactive pools */ +if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || +MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE)) { +if
Re: [libvirt] [PATCH] build: use re-entrant functions in virsh
On 09/05/2012 01:36 AM, Eric Blake wrote: Today's patches pointed out that virsh was still using localtime(), which is not thread-safe, even though virsh is definitely multi-threaded. * cfg.mk (exclude_file_name_regexp--sc_prohibit_nonreentrant): Tighten the rule. * tools/virsh.c (vshOutputLogFile): Avoid localtime. (vshEditWriteToTempFile, vshEditReadBackFile, cmdCd, cmdPwd) (vshCloseLogFile): Avoid strerror. * tools/console.c (vshMakeStdinRaw): Likewise. * tools/virsh-domain.c (vshGenFileName): Fix spacing in previous patch. --- cfg.mk | 2 +- tools/console.c | 11 +++ tools/virsh-domain.c | 2 +- tools/virsh.c| 40 4 files changed, 33 insertions(+), 22 deletions(-) ACK, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: avoid doubled name in syntax check failures
On 09/05/2012 01:41 AM, Eric Blake wrote: Based on a similar gnulib patch; use of $(_sc_search_regexp) already injects $(ME) into any output messages, so a failure of these rules would look like this, pre-patch: ./config.status: ./config.status: use virStrToLong_*, not strtol variants * cfg.mk (sc_prohibit_strncmp, sc_prohibit_strtol) (sc_libvirt_unmarked_diagnostics): Drop redundant name. ACK, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Remove explicit dependency on ceph RPM
From: Daniel P. Berrange berra...@redhat.com The libvirt storage driver uses librbd.so for its functionality. RPM will automatically add a dependency on the library, so there is no need to have an explicit dependency on the ceph RPM itself. This allows newer Fedora distros to avoid pulling in the huge ceph RPM, in favour of just having the libraries installed --- libvirt.spec.in | 4 1 file changed, 4 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 9a1feed..523ac0a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -649,10 +649,6 @@ Requires: device-mapper # For multipath support Requires: device-mapper %endif -%if %{with_storage_rbd} -# For RBD support -Requires: ceph -%endif %if %{with_storage_sheepdog} # For Sheepdog support Requires: sheepdog -- 1.7.11.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V4] implement offline migration
On Wed, Sep 05, 2012 at 08:51:05AM +0800, liguang wrote: 在 2012-09-04二的 12:12 +0100,Daniel P. Berrange写道: On Mon, Sep 03, 2012 at 02:23:24PM +0800, liguang wrote: allow migration even domain isn't active by inserting some stubs to tunnel migration path. Signed-off-by: liguang lig.f...@cn.fujitsu.com --- src/qemu/qemu_driver.c|2 +- src/qemu/qemu_migration.c | 181 +++-- src/qemu/qemu_migration.h |3 +- 3 files changed, 178 insertions(+), 8 deletions(-) I really don't like the general design of this patch, even ignoring all the code bugs. I think this entire patch is really just a solution in search of a problem. Offline migration is already possible with existing libvirt APIs: domsrc = virDomainLookupByName(connsrc, someguest); xml = virDomainGetXMLDesc(domsrc); domdst virDomainDefine(conndst, xml); Um, maybe you mean offline migration is just redefinition of domain at target side, but what about disk images the domain used without sharing files between source and target, do we have to take a look at this case? Which can also be done already virStorageVolDownload + virStorageVolUpload which lets the app choose exactly which disks images they wish to copy, rather than assuming all of them should be copied. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Add cpu hotplug support to libvirt.
On 09/05/2012 07:32 AM, Tang Chen wrote: 4) Make libvirt not use cpuset cgroup. - For now, seems impossable. sched_setaffinity() behaves properly, which assumes the repluged cpu is the same one unpluged before. (am I right ?) But with cgroup's control, we cannot resolve this problem using sched_setaffinity(). If I want to solve the start failure problem, what should I do ? Hi, I posted a comment some time ago about that. If you do not mount the cpuset controller, i.e for RHEL 6 you delete the cpuset line from /etc/cgconfig, the CPU affinity isn't controlled by cgroups any more but uses the old mechanism, which works as expected: take a host CPU offline and it will be removed from the process CPU mask and will show up again after onlining the host CPU. The only issue I currently see is that the display of virsh vcpuinfo and vcpupin is somewhat strange. Using taskset will however show the the correct affinity. I suggest that you try out that approach. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V4] implement offline migration
On Mon, Sep 03, 2012 at 02:23:24PM +0800, liguang wrote: allow migration even domain isn't active by inserting some stubs to tunnel migration path. Signed-off-by: liguang lig.f...@cn.fujitsu.com --- src/qemu/qemu_driver.c|2 +- src/qemu/qemu_migration.c | 181 +++-- src/qemu/qemu_migration.h |3 +- 3 files changed, 178 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d74bf52..00ca211 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9779,7 +9779,7 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn, virCheckFlags(QEMU_MIGRATION_FLAGS, -1); -if (!dom_xml) { +if (!dom_xml !(flags VIR_MIGRATE_OFFLINE)) { This is bogus, XML should be required no matter what. @@ -721,6 +747,12 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig, qemuMigrationCookieAddPersistent(mig, dom) 0) return -1; +if (flags QEMU_MIGRATION_COOKIE_OFFLINE) { +mig-flags |= QEMU_MIGRATION_COOKIE_OFFLINE; +mig-offline = 1; 'mig-offline' is just duplicating what's already tracked in flags. @@ -1307,6 +1339,27 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, /* Domain starts inactive, even if the domain XML had an id field. */ vm-def-id = -1; +if (tunnel) { +if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, + QEMU_MIGRATION_COOKIE_OFFLINE))) +return ret; +else if (mig-offline) { +char *file, *str, *tmp = NULL; +ret = 0; +for (str = mig-mig_file; ; str = NULL) { +file = strtok_r(str, , tmp); Encoding multiple filenames in a single string like this is pure evil and a security flaw waiting to happen. eg, consider how well this works if the guest filename contains a space character. +if (file == NULL) +break; +if (virFDStreamCreateFile(st, file, 0, 0, O_WRONLY, 0) 0) { Re-using the same virStreamPtr object for multiple files is an abuse of the stream API. If this actually works it is by pure luck and is certainly not intended to work. +/* + * do offline migration + */ +static int doMigrateOffline(struct qemud_driver *driver, + virConnectPtr dconn, + virDomainObjPtr vm, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + const char *dom_xml, + const char *dname, + virStreamPtr st, + unsigned long resource) +{ +xmlDocPtr xml = NULL; +xmlXPathContextPtr ctxt = NULL; +xmlNodePtr *disks = NULL; +qemuMigrationCookiePtr mig = NULL; +virBuffer buf = VIR_BUFFER_INITIALIZER; +int i = 0, cn = 0, fd = -1, ret = -1; +char *src[] = {NULL}, *files = NULL; + +VIR_DEBUG(driver=%p, vm=%p, st=%p, cookiein=%s, cookieinlen=%d, + cookieout=%p, cookieoutlen=%p, dom_xml=%s, + dname=%s, flags=%lx, resource=%lu, + driver, vm, st, NULLSTR(cookiein), cookieinlen, + cookieout, cookieoutlen, dom_xml, dname, flags, resource); + +xml = virXMLParseStringCtxt(dom_xml, _((domain_definition)), ctxt); +if (!xml) { + virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(can't parse dom_xml for offline migration \n)); +goto cleanup; +} +cn = virXPathNodeSet(./devices/disk, ctxt, disks); +if (cn 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Fail to get disk node\n)); +goto cleanup; +} +cn = 1; + +for (i = 0 ; i cn ; i++) { +ctxt-node = disks[i]; +src[i] = virXPathString(string(./source/@file +|./source/@dir +|./source/@name), ctxt); +virBufferAsprintf(buf, %s , src[i]); +} Writing custom XML parsing code is forbidden - we have APIs for doing this. I don't really know how you're expecting this work at all when 'source/@dir' is in the XML, since this refers to a directory tree, not a single file you're not recursively copying the directory. Likewise this can't possibly work with ./source/@name since that refers to a network block device which you can't simply open as if it were a local file. +if (qemuMigrationBakeCookie(mig, driver, vm, +cookieout, cookieoutlen, +QEMU_MIGRATION_COOKIE_OFFLINE) 0) +goto cleanup; + +cookiein = *cookieout; +cookieinlen = *cookieoutlen; +cookieout = NULL; +cookieoutlen
[libvirt] Libvir JNA report SIGSEGV
Hi, I try to verify the JNA with concurrent situation but meet some problems. The following is my example code: public static void testcase1() throws LibvirtException { Connect conn=null; Connect conn1=null; //connect to the hypervisor conn = new Connect(esx://10.74.125.68:443/?no_verify=1transport=https, new ConnectAuthDefault(), 0); System.out.println(conn.getVersion()); //connect to the hypervisor conn1 = new Connect(esx://10.74.125.90:443/?no_verify=1transport=https, new ConnectAuthDefault(), 0); System.out.println(conn1.getVersion()); while(true) { int[] array = new int[1]; Long version = conn.getVersion(); Long version1 = conn1.getVersion(); try { Thread.sleep(1000); } catch(Exception e) { } } } When I add line int[] array = new int[1], then the following error will be generated very quickly: # An unexpected error has been detected by Java Runtime Environment: # # SIGSEGV (0xb) at pc=0x003f9b07046e, pid=30049, tid=1109510464 # # Java VM: OpenJDK 64-Bit Server VM (1.6.0-b09 mixed mode linux-amd64) # Problematic frame: # C [libc.so.6+0x7046e] # # An error report file with more information is saved as: I have tried to write the similar code as following. It works well. static void virXenBasic_TC001(void) { virConnectPtr conn = NULL; virConnectPtr conn1 = NULL; unsigned long version = 0; unsigned long version1 = 0; char *hostname = NULL; conn = virConnectOpenAuth(esx://10.74.125.21/?no_verify=1, virConnectAuthPtrDefault, 0); if (conn == NULL) { fprintf(stderr, Failed to open connection to qemu:///system\n); return; } conn1 = virConnectOpenAuth(esx://192.168.119.40/?no_verify=1, virConnectAuthPtrDefault, 0); if (conn1 == NULL) { fprintf(stderr, Failed to open connection to qemu:///system\n); return; } while(true) { hostname = malloc(sizeof(char) * 1); virConnectGetVersion(conn, version); virConnectGetVersion(conn, version1); free(hostname); sleep(1); } return; } B.R. Benjamin Wang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Add cpu hotplug support to libvirt.
Hi Viktor, On 09/05/2012 04:54 PM, Viktor Mihajlovski wrote: I posted a comment some time ago about that. If you do not mount the cpuset controller, i.e for RHEL 6 you delete the cpuset line from /etc/cgconfig, the CPU affinity isn't controlled by cgroups any more but uses the old mechanism, which works as expected: take a host CPU offline and it will be removed from the process CPU mask and will show up again after onlining the host CPU. The only issue I currently see is that the display of virsh vcpuinfo and vcpupin is somewhat strange. Using taskset will however show the the correct affinity. I suggest that you try out that approach. I saw your comment before. You are quite right. :) But the situation here is there are some other features in libvirt using cpuset. For example, emulator-pin feature. If we remove cpuset in the system, other features could be unusable. And more, I found different cgroups are widely used in libvirt now. I don't think removing cgroups from system is a good enough idea, though it can be a work around. What do you think? :) Thanks. :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer][PATCH 1/4] Load osinfo DB on init
as we need this DB later to find an OS or hypervisor and supported devices. --- libvirt-designer/Makefile.am |1 + libvirt-designer/libvirt-designer-domain.c |5 +++- libvirt-designer/libvirt-designer-internal.h | 30 ++ libvirt-designer/libvirt-designer-main.c | 17 +- 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 libvirt-designer/libvirt-designer-internal.h diff --git a/libvirt-designer/Makefile.am b/libvirt-designer/Makefile.am index cf40419..8f10c41 100644 --- a/libvirt-designer/Makefile.am +++ b/libvirt-designer/Makefile.am @@ -20,6 +20,7 @@ DESIGNER_GENERATED_FILES = \ DESIGNER_HEADER_FILES = \ libvirt-designer.h \ + libvirt-designer-internal.h \ libvirt-designer-main.h \ libvirt-designer-domain.h \ $(NULL) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 9b4a7ed..a8cabde 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -17,13 +17,16 @@ * License along with this library; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * - * Author: Daniel P. Berrange berra...@redhat.com + * Authors: + * Daniel P. Berrange berra...@redhat.com + * Michal Privoznik mpriv...@redhat.com */ #include config.h #include sys/utsname.h #include libvirt-designer/libvirt-designer.h +#include libvirt-designer/libvirt-designer-internal.h #define GVIR_DESIGNER_DOMAIN_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_DESIGNER_TYPE_DOMAIN, GVirDesignerDomainPrivate)) diff --git a/libvirt-designer/libvirt-designer-internal.h b/libvirt-designer/libvirt-designer-internal.h new file mode 100644 index 000..bbef922 --- /dev/null +++ b/libvirt-designer/libvirt-designer-internal.h @@ -0,0 +1,30 @@ +/* + * libvirt-designer-internal.h: internal definitions just + * used by code from the library + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * http://www.gnu.org/licenses/. + * + * Author: Michal Privoznik mpriv...@redhat.com + */ + +#ifndef __LIBVIRT_DESIGNER_INTERNAL_H__ +#define __LIBVIRT_DESIGNER_INTERNAL_H__ + +extern OsinfoLoader *osinfo_loader; +extern OsinfoDb *osinfo_db; + +#endif /* __LIBVIRT_DESIGNER_INTERNAL_H__ */ diff --git a/libvirt-designer/libvirt-designer-main.c b/libvirt-designer/libvirt-designer-main.c index 60bf8f5..f2381a6 100644 --- a/libvirt-designer/libvirt-designer-main.c +++ b/libvirt-designer/libvirt-designer-main.c @@ -17,7 +17,9 @@ * License along with this library; If not, see * http://www.gnu.org/licenses/. * - * Author: Daniel P. Berrange berra...@redhat.com + * Authors: + * Daniel P. Berrange berra...@redhat.com + * Michal Privoznik mpriv...@redhat.com */ #include config.h @@ -28,6 +30,9 @@ #include libvirt-designer/libvirt-designer.h #include libvirt-gconfig/libvirt-gconfig.h +OsinfoLoader *osinfo_loader = NULL; +OsinfoDb *osinfo_db = NULL; + /** * gvir_designer_init: * @argc: (inout): pointer to application's argc @@ -80,5 +85,15 @@ gboolean gvir_designer_init_check(int *argc, gvir_log_handler, NULL); #endif +/* Init libosinfo and load databases from default paths */ +/* XXX maybe we want to let users tell a different path via + * env variable or argv */ +osinfo_loader = osinfo_loader_new(); +osinfo_loader_process_default_path(osinfo_loader, err); +if (err) +return FALSE; + +osinfo_db = osinfo_loader_get_db(osinfo_loader); + return TRUE; } -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer][PATCH 3/4] examples: Create an example of usage program
--- .gitignore |1 + Makefile.am |2 +- configure.ac |6 +- examples/Makefile.am | 23 examples/virtxml.c | 334 ++ 5 files changed, 364 insertions(+), 2 deletions(-) create mode 100644 examples/Makefile.am create mode 100644 examples/virtxml.c diff --git a/.gitignore b/.gitignore index b7ba45a..d570af8 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ Makefile.in /config.log /config.status /configure +/examples/virtxml /libtool /libvirt-designer-1.0.pc /libvirt-designer.spec diff --git a/Makefile.am b/Makefile.am index b0f68c0..ab06626 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,5 +1,5 @@ -SUBDIRS = libvirt-designer +SUBDIRS = libvirt-designer examples ACLOCAL_AMFLAGS = -I m4 diff --git a/configure.ac b/configure.ac index 795990f..ebe7b35 100644 --- a/configure.ac +++ b/configure.ac @@ -13,6 +13,7 @@ AM_SILENT_RULES([yes]) LIBOSINFO_REQUIRED=0.0.5 LIBVIRT_GCONFIG_REQUIRED=0.0.9 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 +LIBVIRT_REQUIRED=0.9.0 LIBVIRT_DESIGNER_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` LIBVIRT_DESIGNER_MINOR_VERSION=`echo $VERSION | awk -F. '{print $2}'` @@ -40,6 +41,7 @@ LIBVIRT_DESIGNER_COMPILE_WARNINGS PKG_CHECK_MODULES(LIBOSINFO, libosinfo-1.0 = $LIBOSINFO_REQUIRED) PKG_CHECK_MODULES(LIBVIRT_GCONFIG, libvirt-gconfig-1.0 = $LIBVIRT_GCONFIG_REQUIRED) +PKG_CHECK_MODULES(LIBVIRT, libvirt = $LIBVIRT_REQUIRED) LIBVIRT_DESIGNER_GETTEXT LIBVIRT_DESIGNER_GTK_MISC @@ -51,7 +53,8 @@ LIBVIRT_DESIGNER_INTROSPECTION AC_OUTPUT(Makefile libvirt-designer/Makefile libvirt-designer.spec - libvirt-designer-1.0.pc) + libvirt-designer-1.0.pc + examples/Makefile) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Configuration summary]) @@ -62,4 +65,5 @@ AC_MSG_NOTICE([ Libraries:]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ LIBOSINFO: $LIBOSINFO_CFLAGS $LIBOSINFO_LIBS]) AC_MSG_NOTICE([ LIBVIRT_GCONFIG: $LIBVIRT_GCONFIG_CFLAGS $LIBVIRT_GCONFIG_LIBS]) +AC_MSG_NOTICE([ LIBVIRT: $LIBVIRT_CFLAGS $LIBVIRT_LIBS]) AC_MSG_NOTICE([]) diff --git a/examples/Makefile.am b/examples/Makefile.am new file mode 100644 index 000..cc2b997 --- /dev/null +++ b/examples/Makefile.am @@ -0,0 +1,23 @@ +INCLUDES = \ + -I$(top_builddir)/libvirt-designer \ + -I$(top_srcdir) + +virtxml_LDADD = \ + $(top_builddir)/libvirt-designer/libvirt-designer-1.0.la + +virtxml_CFLAGS = \ + -I$(top_srcdir) \ + $(COVERAGE_CFLAGS) \ + -I$(top_srcdir) \ + $(LIBOSINFO_CFLAGS) \ + $(LIBVIRT_GCONFIG_CFLAGS) \ + $(WARN_CFLAGS2) \ + $(LIBVIRT_CFLAGS) \ + $(NULL) + +virtxml_LDFLAGS = \ + $(LIBOSINFO_LIBS) \ + $(LIBVIRT_GCONFIG_LIBS) \ + $(LIBVIRT_LIBS) + +bin_PROGRAMS = virtxml diff --git a/examples/virtxml.c b/examples/virtxml.c new file mode 100644 index 000..36bc3bb --- /dev/null +++ b/examples/virtxml.c @@ -0,0 +1,334 @@ +/* + * virtxml.c: produce an domain XML + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * http://www.gnu.org/licenses/. + * + * Author: Michal Privoznik mpriv...@redhat.com + */ + +#include config.h +#include libvirt-designer/libvirt-designer.h +#include libvirt/libvirt.h +#include libvirt/virterror.h + +#include stdio.h +#include getopt.h +#include stdlib.h +#include stdarg.h +#include string.h +#include sys/types.h +#include sys/stat.h +#include unistd.h + +#define print_error(...) \ +print_error_impl(__FUNCTION__, __LINE__, __VA_ARGS__) + +static void +print_error_impl(const char *funcname, + size_t linenr, + const char *fmt, ...) +{ +va_list args; + +fprintf(stderr, Error in %s:%zu , funcname, linenr); +va_start(args, fmt); +vfprintf(stderr, fmt, args); +va_end(args); +fprintf(stderr,\n); +} + +static void +print_usage(const char *progname) +{ +printf(\nUsage: %s options ...\n +options:\n + -h | --help print this help\n + -c | --connect=URI libvirt connection URI used \n + for querying capabilities\n + --list-os
[libvirt] [libvirt-designer][PATCH 4/4] domain: Introduce interface support
Let users add NICs to domains. --- examples/virtxml.c | 96 libvirt-designer/libvirt-designer-domain.c | 53 +++ libvirt-designer/libvirt-designer-domain.h |3 + libvirt-designer/libvirt-designer.sym |1 + 4 files changed, 141 insertions(+), 12 deletions(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index 36bc3bb..87929b2 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -56,17 +56,19 @@ print_usage(const char *progname) { printf(\nUsage: %s options ...\n options:\n - -h | --help print this help\n - -c | --connect=URI libvirt connection URI used \n - for querying capabilities\n - --list-os list IDs of known OSes\n - --list-platformlist IDs of known hypervisors\n - -o | --os=OSset domain OS\n - -p | --platform=PLATFORMset hypervisor under which \n - domain will be running\n - -a | --architecture=ARCHset domain architecture\n - -d | --disk=PATH[,FORMAT] add disk to domain with PATH being \n - source and FORMAT is its format\n, + -h | --help print this help\n + -c | --connect=URIlibvirt connection URI used \n + for querying capabilities\n + --list-oslist IDs of known OSes\n + --list-platform list IDs of known hypervisors\n + -o | --os=OS set domain OS\n + -p | --platform=PLATFORM set hypervisor under which \n + domain will be running\n + -a | --architecture=ARCH set domain architecture\n + -d | --disk=PATH[,FORMAT] add disk to domain with PATH being \n + source and FORMAT is its format\n + -i | --interface=NETWORK[,ARG=VAL]add interface with NETWORK source.\n + Possible ARGs: mac,link={up,down}\n, progname); } @@ -186,6 +188,69 @@ add_disk(gpointer data, } +static void +add_iface(gpointer data, + gpointer user_data) +{ +GVirDesignerDomain *domain = (GVirDesignerDomain *) user_data; +char *network = (char *) data; +char *param = NULL; +GVirConfigDomainInterface *iface = NULL; +GError *error = NULL; + +param = strchr(network, ','); +if (param) { +*param = '\0'; +param++; +} + +iface = gvir_designer_domain_add_interface_network(domain, network, error); +if (error) { +print_error(%s, error-message); +exit(EXIT_FAILURE); +} + +while (param *param) { +char *key = param; +char *val; +GVirConfigDomainInterfaceLinkState link; + +/* move to next token */ +param = strchr(param, ','); +if (param) { +*param = '\0'; +param++; +} + +/* parse token */ +val = strchr(key, '='); +if (!val) { +print_error(Invalid format: %s, key); +exit(EXIT_FAILURE); +} + +*val = '\0'; +val++; + +if (!strcmp(key, mac)) { +gvir_config_domain_interface_set_mac(iface, val); +} else if (!strcmp(key, link)) { +if (!strcmp(val, up)) { +link = GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_UP; +} else if (!strcmp(val, down)) { +link = GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DOWN; +} else { +print_error(Unknown value: %s, val); +exit(EXIT_FAILURE); +} +gvir_config_domain_interface_set_link_state(iface, link); +} else { +print_error(Unknown key: %s, key); +exit(EXIT_FAILURE); +} +} +} + #define CHECK_ERROR \ if (error) {\ print_error(%s, error-message); \ @@ -210,6 +275,7 @@ main(int argc, char *argv[]) char *arch_str = NULL; char *connect_uri = NULL; GList *disk_str_list = NULL; +GList *iface_str_list = NULL; int arg; struct option opt[] = { @@ -221,6 +287,7 @@ main(int argc, char *argv[]) {platform, required_argument, NULL, 'p'}, {architecture, required_argument, NULL, 'a'}, {disk, required_argument, NULL, 'd'}, +{interface, required_argument, NULL, 'i'}, {NULL, 0, NULL, 0} }; @@ -230,7 +297,7 @@ main(int argc, char *argv[]) /* Standard (non-command) options. The leading + ensures that no
[libvirt] [libvirt-designer][PATCH 0/4] Basic disk interface support
with small example called virtxml Michal Privoznik (4): Load osinfo DB on init domain: Introduce disk support examples: Create an example of usage program domain: Introduce interface support .gitignore |1 + Makefile.am |2 +- configure.ac |6 +- examples/Makefile.am | 23 ++ examples/virtxml.c | 406 ++ libvirt-designer/Makefile.am |1 + libvirt-designer/libvirt-designer-domain.c | 302 +++- libvirt-designer/libvirt-designer-domain.h | 12 +- libvirt-designer/libvirt-designer-internal.h | 30 ++ libvirt-designer/libvirt-designer-main.c | 17 +- libvirt-designer/libvirt-designer.sym|4 + 11 files changed, 799 insertions(+), 5 deletions(-) create mode 100644 examples/Makefile.am create mode 100644 examples/virtxml.c create mode 100644 libvirt-designer/libvirt-designer-internal.h -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer][PATCH 2/4] domain: Introduce disk support
Let users add either files or devices as disks to domains. --- libvirt-designer/libvirt-designer-domain.c | 244 libvirt-designer/libvirt-designer-domain.h |7 + libvirt-designer/libvirt-designer.sym |3 + 3 files changed, 254 insertions(+), 0 deletions(-) diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index a8cabde..20611f2 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -37,6 +37,12 @@ struct _GVirDesignerDomainPrivate GVirConfigCapabilities *caps; OsinfoOs *os; OsinfoPlatform *platform; + +OsinfoDeployment *deployment; +/* next disk targets */ +unsigned int ide; +unsigned int virtio; +unsigned int sata; }; G_DEFINE_TYPE(GVirDesignerDomain, gvir_designer_domain, G_TYPE_OBJECT); @@ -134,6 +140,7 @@ static void gvir_designer_domain_finalize(GObject *object) g_object_unref(priv-os); g_object_unref(priv-platform); g_object_unref(priv-caps); +g_object_unref(priv-deployment); G_OBJECT_CLASS(gvir_designer_domain_parent_class)-finalize(object); } @@ -663,3 +670,240 @@ cleanup: g_object_unref(guest); return ret; } + +static GList * +gvir_designer_domain_get_supported_disk_bus_types(GVirDesignerDomain *design) +{ +GVirDesignerDomainPrivate *priv = design-priv; +OsinfoDeviceList *dev_list; +GHashTable *bus_hash = g_hash_table_new(g_str_hash, g_str_equal); +GList *ret = NULL; +int i; + +dev_list = osinfo_os_get_devices_by_property(priv-os, class, block, TRUE); + +for (i = 0; i osinfo_list_get_length(OSINFO_LIST(dev_list)); i++) { +OsinfoDevice *dev = OSINFO_DEVICE(osinfo_list_get_nth(OSINFO_LIST(dev_list), i)); +const gchar *bus = osinfo_device_get_bus_type(dev); + +if (bus) +g_hash_table_add(bus_hash, g_strdup(bus)); +} + +ret = g_hash_table_get_keys(bus_hash); +ret = g_list_copy(ret); + +g_hash_table_destroy(bus_hash); +return ret; +} + +static OsinfoDeviceLink * +gvir_designer_domain_get_preferred_device(GVirDesignerDomain *design, + const char *class, + GError **error) +{ +GVirDesignerDomainPrivate *priv = design-priv; +OsinfoFilter *filter = osinfo_filter_new(); +OsinfoDeviceLinkFilter *filter_link = NULL; +OsinfoDeployment *deployment = priv-deployment; +OsinfoDeviceLink *dev_link = NULL; + +if (!deployment) { +priv-deployment = deployment = osinfo_db_find_deployment(osinfo_db, + priv-os, + priv-platform); +if (!deployment) { +g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0, +Unable to find any deployment in libosinfo database); +goto cleanup; +} +} + +osinfo_filter_add_constraint(filter, class, class); +filter_link = osinfo_devicelinkfilter_new(filter); +dev_link = osinfo_deployment_get_preferred_device_link(deployment, OSINFO_FILTER(filter_link)); + +cleanup: +if (filter_link) +g_object_unref(filter_link); +if (filter) +g_object_unref(filter); +return dev_link; +} + +static const gchar * +gvir_designer_domain_get_preferred_disk_bus_type(GVirDesignerDomain *design, + GError **error) +{ +OsinfoDevice *dev; +OsinfoDeviceLink *dev_link = gvir_designer_domain_get_preferred_device(design, + block, + error); +const gchar *ret = NULL; + +if (!dev_link) +return NULL; + +dev = osinfo_devicelink_get_target(dev_link); +ret = osinfo_device_get_bus_type(dev); + +return ret; +} + +static gchar * +gvir_designer_domain_next_disk_target(GVirDesignerDomain *design, + GVirConfigDomainDiskBus bus) +{ +gchar *ret = NULL; +GVirDesignerDomainPrivate *priv = design-priv; + +switch (bus) { +case GVIR_CONFIG_DOMAIN_DISK_BUS_IDE: +ret = g_strdup_printf(hd%c, 'a' + priv-ide++); +break; +case GVIR_CONFIG_DOMAIN_DISK_BUS_VIRTIO: +ret = g_strdup_printf(vd%c, 'a' + priv-virtio++); +break; +case GVIR_CONFIG_DOMAIN_DISK_BUS_SATA: +ret = g_strdup_printf(sd%c, 'a' + priv-sata++); +break; +case GVIR_CONFIG_DOMAIN_DISK_BUS_FDC: +case GVIR_CONFIG_DOMAIN_DISK_BUS_SCSI: +case GVIR_CONFIG_DOMAIN_DISK_BUS_XEN: +case GVIR_CONFIG_DOMAIN_DISK_BUS_USB: +case GVIR_CONFIG_DOMAIN_DISK_BUS_UML: +default: +/* not supported yet */ +/* XXX should we fallback to ide/virtio? */ +break; +} + +return
Re: [libvirt] [PATCH] Remove explicit dependency on ceph RPM
On Wed, Sep 05, 2012 at 09:00:20 +0100, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The libvirt storage driver uses librbd.so for its functionality. RPM will automatically add a dependency on the library, so there is no need to have an explicit dependency on the ceph RPM itself. This allows newer Fedora distros to avoid pulling in the huge ceph RPM, in favour of just having the libraries installed --- libvirt.spec.in | 4 1 file changed, 4 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 9a1feed..523ac0a 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -649,10 +649,6 @@ Requires: device-mapper # For multipath support Requires: device-mapper %endif -%if %{with_storage_rbd} -# For RBD support -Requires: ceph -%endif %if %{with_storage_sheepdog} # For Sheepdog support Requires: sheepdog ACK, all the auto-dependency magic is enabled with existing BuildRequires: ceph-devel. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] S4 event, states and caveats
Hi everybody, I'm going to add support for S4 event for libvirt and I'm thinking for a while now how to do that, so I'd like to discuss this here on the list before doing something wrong and mainly to eliminate bothering you guys with reviews of PATCH v5 etc. QEMU emits SUSPEND_DISK event before SHUTDOWN and then shuts down as usual when the guest is powering off. At first I wanted to add it the same way the support for SUSPEND was added, but when I come to think of it, this needs a little bit more. My main questions are: - What state should the machine have after it stops? a) pmsuspended (optionally with new reason) b) shutoff (ditto) c) new state (pmsuspended-disk for example) - How should we behave when dompmwakeup is called for such domain? - How should we behave when destroy is called for such domain? - How do we store the domain state persistently between restarts? - Should there be new lifecycle event to distinguish suspend to disk from poweroff? Plus you might have some additional things what to care about. In that case please let me know so it's been dealt with at the start. I'm inclining to new state for the domain (just because suspend to disk doesn't seem like a new reason to me) which can be woken up by the pmwakeup call (that would be basically the same as starting it). It's hard for me to say if destroy should reset the state. Even when it is prohibited, the machine can be recreated with the same disk and thus achieving the same result with a workaround. We can probably allow removing and changing devices connected as well as other settings of the domain. It is then problem of QEMU, it's BIOS and mainly the guest OS to deal with it, so basically it can be blamed on the user who did that :) If we don't allow that, the same rule with the workaround applies here anyway. I know Michal was thinking about how to store the data for the domain state because of transient devices etc. His idea was to save the files somewhere else than /var/run. I'd do that, but just for those suspended to disk. I'd add a lifecycle event with lifecycle control in the domain XML (now supported for poweroff, reboot and crash). Thanks for any remarks, additions, hints and replies. Have a nice day, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] [PATCH v2 1/6] add configure option --with-fuse for libvirt
Hi Daniel Glauber 于 2012年07月31日 17:27, Daniel P. Berrange 写道: Hi Gao, I'm wondering if you are planning to attend the Linux Plumbers Conference in San Diego at the end of August ? Glauber is going to be giving a talk on precisely the subject of virtualizing /proc in containers which is exactly what your patch is looking at https://blueprints.launchpad.net/lpc/+spec/lpc2012-cont-proc I'll review your patches now, but I think I'd like to wait to hear what Glauber talks about at LPC before we try to merge this support in libvirt, so we have an broadly agreed long term strategy for /proc between all the interested userspace kernel guys. I did not attend the LPC,so can you tell me what's the situation of the /proc virtualization? I think maybe we should just apply this patchset first,and wait for somebody sending patches to implement /proc virtualization. Thanks Gao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Add cpu hotplug support to libvirt.
On Wed, Sep 05, 2012 at 01:32:12PM +0800, Tang Chen wrote: Hi Srivatsa, Daniel, Thank you very much for all the comments. :) On 09/05/2012 04:57 AM, Srivatsa S. Bhat wrote: I had posted a Linux kernel patchset[1] some time ago to expose another file so that we can distinguish between the user specified settings vs the actual scenario underneath. But the conclusion in the ensuing discussion was that the existing kernel behaviour is good as is, and trying to fix it would break kernel semantics. (However, note that the suspend/resume case has been fixed in the kernel by commit d35be8bab). [1]. http://thread.gmane.org/gmane.linux.documentation/4805 The reason why I made this patch set is that if libvirt doesn't recover the cpuset.cpus, all the domains with vcpus pinned to a *re-pluged* cpu in xml will fail to start. Which means all these domain will be unusable, or we have to modify the configuration. If the cpu is really removed, it is normal for a domain fails to start. We can simply print an error message. But if the cpu is added again, and it is active and usable, the domain should be able to start normally. (am I right here ?) This is the key problem I want to solve. So first, I improved the netlink related code in libvirt, and now libvirt can be notified when cpu hotplug event occurred. Your patch appears to work in some limited scenarios, but more generally it will fail to work, and resulted in undesirable behaviour. Consider for example, if libvirtd is configured thus: cd /sys/fs/cgroup/cpuset mkdir demo cd demo echo 2-3 cpuset.cpus echo 0 cpuset.mems echo $$ tasks /usr/sbin/libvirtd ie, libvirtd is now running on cpus 2-3, in group 'demo'. VMs will be created in /sys/fs/cgroup/cpuset/demo/libvirt/qemu/$VMNAME Your patch attempts to set the cpuset.cpus on 'libvirt/qemu/$VMNAME' but ignores the fact that there could be many higher directories (eg demo here) that need setting. libvirtd, however, should not be responsible for / allowed to change settings in parent cgroups from where it was started. ie in this example, libvirtd should *not* touch the 'demo' cgroup. So consider systemd starting tasks, giving them custom cgroups. Now systemd also has to listen for netlink events and reset the cpuset masks. Things are even worse if the admin has temporarily offlined all the cpus that are associated with the current cpuset. When this happens the kernel throws libvirtd and all its VMs out of their current cgroups and dumps them up in a parent cgroup (potentially even the root group). This is really awful. I read the emails posted above. In summary, you discussed about the following problems: 1) Make cgroup be able to distinguish actual configuration and user's. - ( Srivatsa's idea: mask = (actual config) (user config) ) Seems that it is hard to be applied for some cgroup design reasons. 2) Kill all the tasks on the cpu when hot-unplug it. - I don't think this is a good idea. And, this won't solve the problem. For example, a task binded on cpu 3. Suppose cpu 3 is unpluged, * if the task is killed, it's just too rude, and users running important tasks will suffer. * if the task is migrated to other cpus, what if cpu 3 is active again ? Are we going to see the added cpu 3 is not the original cpu 3 ? Whatever, the domain will still fail to start. IMHO, execution of those tasks should simply be paused (same way that the 'freezer' cgroup pauses tasks). The admin can then either move the tasks to an alternate cgroup, or change the cpuset mask to allow them to continue running. The kernel's current behaviour of pushing all tasks up into a parent cgroup is just crazy - it is just throwing away the users requested cpu mask forever :-( 3) Make cpu hot unplug fail when there are tasks on it. - This may be unacceptable for hotplug users. And this won't solve the problem either. If the domain is not running when the hot unplug happens, the hot unplug will succeed. And when we start the domain, it will fail anyway, right ? 4) Make libvirt not use cpuset cgroup. - For now, seems impossable. sched_setaffinity() behaves properly, which assumes the repluged cpu is the same one unpluged before. (am I right ?) But with cgroup's control, we cannot resolve this problem using sched_setaffinity(). If I want to solve the start failure problem, what should I do ? I maintain the problems we see with cpuset controller cannot be reasonably solved by libvirtd, or userspace in general. The kernel behaviour is just flawed. If the kernel won't fix it, then we should recommend people not to use the cpuset cgroup at all, and just rely on our sched_setaffinity support instead. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org
Re: [libvirt] [PATCH v2 0/4] Add cpu hotplug support to libvirt.
On Wed, Sep 05, 2012 at 05:19:22PM +0800, Tang Chen wrote: Hi Viktor, On 09/05/2012 04:54 PM, Viktor Mihajlovski wrote: I posted a comment some time ago about that. If you do not mount the cpuset controller, i.e for RHEL 6 you delete the cpuset line from /etc/cgconfig, the CPU affinity isn't controlled by cgroups any more but uses the old mechanism, which works as expected: take a host CPU offline and it will be removed from the process CPU mask and will show up again after onlining the host CPU. The only issue I currently see is that the display of virsh vcpuinfo and vcpupin is somewhat strange. Using taskset will however show the the correct affinity. I suggest that you try out that approach. I saw your comment before. You are quite right. :) But the situation here is there are some other features in libvirt using cpuset. For example, emulator-pin feature. If we remove cpuset in the system, other features could be unusable. The emulator pinning code should be made to use sched_setaffinity() when the cpuset controller is not available, just as we do for the vCPU pinning. And more, I found different cgroups are widely used in libvirt now. I don't think removing cgroups from system is a good enough idea, though it can be a work around. What do you think? :) Either the kernel fixes the broken cpuset behaviour, or we have to recommend that people don't use it. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] parallels: remove domains list from _parallelsConn structure
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c |5 - src/parallels/parallels_utils.h |1 - 2 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 72e32ff..96a8ab1 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -671,9 +671,6 @@ parallelsOpenDefault(virConnectPtr conn) if (!(privconn-caps = parallelsBuildCapabilities())) goto error; -if (!(privconn-domains = parallelsGetDomains(privconn))) -goto error; - conn-privateData = privconn; return VIR_DRV_OPEN_SUCCESS; @@ -725,8 +722,6 @@ parallelsClose(virConnectPtr conn) parallelsDriverLock(privconn); virCapabilitiesFree(privconn-caps); -virDomainObjListDeinit(privconn-domains); -VIR_FREE(privconn-domains); conn-privateData = NULL; parallelsDriverUnlock(privconn); diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 3468c1d..16a880f 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -32,7 +32,6 @@ struct _parallelsConn { virMutex lock; -virDomainObjListPtr domains; virStoragePoolObjList pools; virCapsPtr caps; virDomainEventStatePtr domainEventState; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] parallels: rename uuidstr to __uuidstr in macro
Rename the variable in macro to avoid warnings declaration of 'uuidstr' shadows a previous local in the future. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 794d61d..8b83a9d 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -60,12 +60,12 @@ #define PRLSRVCTL prlsrvctl #define PARALLELS_DEFAULT_ARCH x86_64 -#define parallelsDomNotFoundError(domain)\ -do { \ -char uuidstr[VIR_UUID_STRING_BUFLEN];\ -virUUIDFormat(domain-uuid, uuidstr);\ -virReportError(VIR_ERR_NO_DOMAIN,\ - _(no domain with matching uuid '%s'), uuidstr); \ +#define parallelsDomNotFoundError(domain) \ +do { \ +char __uuidstr[VIR_UUID_STRING_BUFLEN];\ +virUUIDFormat(domain-uuid, __uuidstr);\ +virReportError(VIR_ERR_NO_DOMAIN, \ + _(no domain with matching uuid '%s'), __uuidstr); \ } while (0) #define parallelsParseError() \ -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] parallels: fix memory allocation
size of videos array must be increased. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 06a75b3..c14f616 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -268,7 +268,7 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value) if (VIR_ALLOC(accel) 0) goto no_memory; -if (VIR_REALLOC_N(def-videos, def-nvideos) 0) +if (VIR_REALLOC_N(def-videos, def-nvideos + 1) 0) goto no_memory; def-videos[def-nvideos++] = video; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] parallels: get rid of cached domains list in driver
The goal of this patch series is to get rid of cached domains list. Someone can create several connections to libvirt or change something (start VM or change parameters) using native tools. In such case first connection will not get any info about those changes. There is no way to check, if the cache valid, so remove it and always query needed data using prlctl command. Dmitry Guryanov (6): parallels: return a new list of domains from parallelsLoadDomains parallels: rename uuidstr to __uuidstr in macro parallels: return up-to-date info about domains parallels: don't track domain state in global domains list parallels: don't use stored domains list in parallelsDomainDefineXML parallels: remove domains list from _parallelsConn structure src/parallels/parallels_driver.c | 423 ++ src/parallels/parallels_utils.h |1 - 2 files changed, 195 insertions(+), 229 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] parallels: return a new list of domains from parallelsLoadDomains
Make parallelsLoadDomains more useful, so that it will not touch list in global _parallelsConn structure, but allocate and return a new one instead. So it can be used later, when you need the up-to-date list of VMs instead of the one stored in _parallelsConn structure. Also rename parallelsLoadDomains to parallelsGetDomains and add new function parallelsGetDomain, which is used in parallelsDomainDefineXML. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 134 +- src/parallels/parallels_utils.h |2 +- 2 files changed, 90 insertions(+), 46 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index c14f616..794d61d 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -401,11 +401,8 @@ parallelsAddVNCInfo(virDomainDefPtr def, virJSONValuePtr jobj_root) return ret; } -/* - * Must be called with privconn-lock held - */ static virDomainObjPtr -parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) +parallelsParseDomainInfo(parallelsConnPtr privconn, virJSONValuePtr jobj) { virDomainObjPtr dom = NULL; virDomainDefPtr def = NULL; @@ -528,11 +525,12 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) if (parallelsAddVNCInfo(def, jobj) 0) goto cleanup; -if (!(dom = virDomainAssignDef(privconn-caps, - privconn-domains, def, false))) +if (!(dom = virDomainObjNew(privconn-caps))) goto cleanup; /* dom is locked here */ +dom-def = def; + dom-privateDataFreeFunc = parallelsDomObjFreePrivate; dom-privateData = pdom; dom-persistent = 1; @@ -562,22 +560,21 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) } /* - * Must be called with privconn-lock held - * * if domain_name is NULL - load information about all * registered domains. */ -static int -parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name) +static virDomainObjListPtr +parallelsGetDomains(parallelsConnPtr privconn) { int count, i; virJSONValuePtr jobj; virJSONValuePtr jobj2; virDomainObjPtr dom = NULL; -int ret = -1; +virDomainObjListPtr domains = NULL; +char uuidstr[VIR_UUID_STRING_BUFLEN]; jobj = parallelsParseOutput(PRLCTL, list, -j, -a, -i, -H, ---vmtype, vm, domain_name, NULL); +--vmtype, vm, NULL); if (!jobj) { parallelsParseError(); goto cleanup; @@ -589,6 +586,11 @@ parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name) goto cleanup; } +if (VIR_ALLOC(domains) 0) +goto error; +if (virDomainObjListInit(domains) 0) +goto error; + for (i = 0; i count; i++) { jobj2 = virJSONValueArrayGet(jobj, i); if (!jobj2) { @@ -596,16 +598,59 @@ parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name) goto cleanup; } -dom = parallelsLoadDomain(privconn, jobj2); +dom = parallelsParseDomainInfo(privconn, jobj2); if (!dom) goto cleanup; + +virUUIDFormat(dom-def-uuid, uuidstr); +if (virHashAddEntry(domains-objs, uuidstr, dom) 0) { +VIR_FREE(dom); +goto error; +} } -ret = 0; + cleanup: +virJSONValueFree(jobj); +return domains; + error: +virJSONValueFree(jobj); +virDomainObjListDeinit(domains); +VIR_FREE(domains); +return NULL; +} + +static virDomainObjPtr +parallelsGetDomain(parallelsConnPtr privconn, const char *domain_name) +{ +int count; +virJSONValuePtr jobj; +virJSONValuePtr jobj2; +virDomainObjPtr dom = NULL; + +jobj = parallelsParseOutput(PRLCTL, list, -j, -a, -i, -H, +--vmtype, vm, domain_name, NULL); +if (!jobj) { +parallelsParseError(); +goto cleanup; +} + +count = virJSONValueArraySize(jobj); +if (count != 1) { +parallelsParseError(); +goto cleanup; +} + +jobj2 = virJSONValueArrayGet(jobj, 0); +if (!jobj2) { +parallelsParseError(); +goto cleanup; +} + +dom = parallelsParseDomainInfo(privconn, jobj2); cleanup: virJSONValueFree(jobj); -return ret; +return dom; } static int @@ -626,18 +671,14 @@ parallelsOpenDefault(virConnectPtr conn) if (!(privconn-caps = parallelsBuildCapabilities())) goto error; -if (virDomainObjListInit(privconn-domains) 0) +if (!(privconn-domains = parallelsGetDomains(privconn))) goto error; conn-privateData = privconn; -if (parallelsLoadDomains(privconn, NULL)) -goto error; - return VIR_DRV_OPEN_SUCCESS; error: -virDomainObjListDeinit(privconn-domains); virCapabilitiesFree(privconn-caps);
[libvirt] [PATCH 1/2] parallels: remove unused member 'os' from parallelsDomObj
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_utils.h |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 082ee75..6a27003 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -44,7 +44,6 @@ typedef struct _parallelsConn *parallelsConnPtr; struct parallelsDomObj { int id; char *uuid; -char *os; }; typedef struct parallelsDomObj *parallelsDomObjPtr; -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] parallels: don't track domain state in global domains list
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 101 ++ 1 files changed, 16 insertions(+), 85 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index de3d53f..1f6dcef 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1098,118 +1098,49 @@ parallelsDomainGetAutostart(virDomainPtr domain, int *autostart) return ret; } -typedef int (*parallelsChangeStateFunc)(virDomainObjPtr privdom); -#define PARALLELS_UUID(x) (((parallelsDomObjPtr)(x-privateData))-uuid) - -static int -parallelsDomainChangeState(virDomainPtr domain, - virDomainState req_state, const char *req_state_name, - parallelsChangeStateFunc chstate, - virDomainState new_state, int reason) -{ -parallelsConnPtr privconn = domain-conn-privateData; -virDomainObjPtr privdom; -int state; -int ret = -1; - -parallelsDriverLock(privconn); -privdom = virDomainFindByUUID(privconn-domains, domain-uuid); -parallelsDriverUnlock(privconn); - -if (privdom == NULL) { -parallelsDomNotFoundError(domain); -goto cleanup; -} - -state = virDomainObjGetState(privdom, NULL); -if (state != req_state) { -virReportError(VIR_ERR_INTERNAL_ERROR, _(domain '%s' not %s), - privdom-def-name, req_state_name); -goto cleanup; -} - -if (chstate(privdom)) -goto cleanup; - -virDomainObjSetState(privdom, new_state, reason); - -ret = 0; - - cleanup: -if (privdom) -virDomainObjUnlock(privdom); - -return ret; -} - -static int parallelsPause(virDomainObjPtr privdom) -{ -return parallelsCmdRun(PRLCTL, pause, PARALLELS_UUID(privdom), NULL); -} - static int parallelsPauseDomain(virDomainPtr domain) { -return parallelsDomainChangeState(domain, - VIR_DOMAIN_RUNNING, running, - parallelsPause, - VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); -} +char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsResume(virDomainObjPtr privdom) -{ -return parallelsCmdRun(PRLCTL, resume, PARALLELS_UUID(privdom), NULL); +virUUIDFormat(domain-uuid, uuidstr); +return parallelsCmdRun(PRLCTL, pause, uuidstr, NULL) ? -1 : 0; } static int parallelsResumeDomain(virDomainPtr domain) { -return parallelsDomainChangeState(domain, - VIR_DOMAIN_PAUSED, paused, - parallelsResume, - VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); -} +char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsStart(virDomainObjPtr privdom) -{ -return parallelsCmdRun(PRLCTL, start, PARALLELS_UUID(privdom), NULL); +virUUIDFormat(domain-uuid, uuidstr); +return parallelsCmdRun(PRLCTL, resume, uuidstr, NULL) ? -1 : 0; } static int parallelsDomainCreate(virDomainPtr domain) { -return parallelsDomainChangeState(domain, - VIR_DOMAIN_SHUTOFF, stopped, - parallelsStart, - VIR_DOMAIN_RUNNING, VIR_DOMAIN_EVENT_STARTED_BOOTED); -} +char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsKill(virDomainObjPtr privdom) -{ -return parallelsCmdRun(PRLCTL, stop, PARALLELS_UUID(privdom), --kill, NULL); +virUUIDFormat(domain-uuid, uuidstr); +return parallelsCmdRun(PRLCTL, start, uuidstr, NULL) ? -1 : 0; } static int parallelsDestroyDomain(virDomainPtr domain) { -return parallelsDomainChangeState(domain, - VIR_DOMAIN_RUNNING, running, - parallelsKill, - VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_DESTROYED); -} +char uuidstr[VIR_UUID_STRING_BUFLEN]; -static int parallelsStop(virDomainObjPtr privdom) -{ -return parallelsCmdRun(PRLCTL, stop, PARALLELS_UUID(privdom), NULL); +virUUIDFormat(domain-uuid, uuidstr); +return parallelsCmdRun(PRLCTL, stop, uuidstr, --kill, NULL) ? -1 : 0; } static int parallelsShutdownDomain(virDomainPtr domain) { -return parallelsDomainChangeState(domain, - VIR_DOMAIN_RUNNING, running, - parallelsStop, - VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_SHUTDOWN); +char uuidstr[VIR_UUID_STRING_BUFLEN]; + +virUUIDFormat(domain-uuid, uuidstr); +return parallelsCmdRun(PRLCTL, stop, uuidstr, NULL) ? -1 : 0; } static int -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] parallels: don't use stored domains list in parallelsDomainDefineXML
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 40 +++-- 1 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 1f6dcef..72e32ff 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1589,8 +1589,9 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) virDomainDefPtr def; virDomainObjPtr dom = NULL, olddom = NULL; int dupVM; +virDomainObjListPtr domains = NULL; +char uuidstr[VIR_UUID_STRING_BUFLEN]; -parallelsDriverLock(privconn); if ((def = virDomainDefParseString(privconn-caps, xml, 1 VIR_DOMAIN_VIRT_PARALLELS, VIR_DOMAIN_XML_INACTIVE)) == NULL) { @@ -1599,41 +1600,32 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } -if ((dupVM = virDomainObjIsDuplicate(privconn-domains, def, 0)) 0) { +if (!(domains = parallelsGetDomains(privconn))) +goto cleanup; + +if ((dupVM = virDomainObjIsDuplicate(domains, def, 0)) 0) { virReportError(VIR_ERR_INVALID_ARG, %s, _(Already exists)); goto cleanup; } if (dupVM == 1) { -olddom = virDomainFindByUUID(privconn-domains, def-uuid); +olddom = virDomainFindByUUID(domains, def-uuid); if (parallelsApplyChanges(olddom, def) 0) { virDomainObjUnlock(olddom); goto cleanup; } virDomainObjUnlock(olddom); - -if (!(dom = virDomainAssignDef(privconn-caps, - privconn-domains, def, false))) { -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Can't allocate domobj)); -goto cleanup; -} - -def = NULL; } else { -char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (parallelsCreateVm(conn, def)) goto cleanup; +} -if (!(dom = parallelsGetDomain(privconn, def-name))) -goto cleanup; +virUUIDFormat(def-uuid, uuidstr); -virUUIDFormat(dom-def-uuid, uuidstr); -if (virHashAddEntry(privconn-domains-objs, uuidstr, dom) 0) { -VIR_FREE(dom); -goto cleanup; -} +if (!(dom = parallelsGetDomain(privconn, uuidstr))) { +virReportError(VIR_ERR_NO_DOMAIN, + _(no domain with matching uuid '%s'), uuidstr); +goto cleanup; } ret = virGetDomain(conn, dom-def-name, dom-def-uuid); @@ -1642,9 +1634,9 @@ parallelsDomainDefineXML(virConnectPtr conn, const char *xml) cleanup: virDomainDefFree(def); -if (dom) -virDomainObjUnlock(dom); -parallelsDriverUnlock(privconn); +virObjectUnref(dom); +virDomainObjListDeinit(domains); +VIR_FREE(domains); return ret; } -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] parallels: return up-to-date info about domains
Use parallelsGetDomain/'s in functions for domain lookup and information obtaining. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- src/parallels/parallels_driver.c | 185 +++-- 1 files changed, 95 insertions(+), 90 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 8b83a9d..de3d53f 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -786,12 +786,16 @@ static int parallelsListDomains(virConnectPtr conn, int *ids, int maxids) { parallelsConnPtr privconn = conn-privateData; -int n; +virDomainObjListPtr domains = NULL; +int n = -1; -parallelsDriverLock(privconn); -n = virDomainObjListGetActiveIDs(privconn-domains, ids, maxids); -parallelsDriverUnlock(privconn); +if (!(domains = parallelsGetDomains(privconn))) +goto cleanup; +n = virDomainObjListGetActiveIDs(domains, ids, maxids); +cleanup: +virDomainObjListDeinit(domains); +VIR_FREE(domains); return n; } @@ -799,12 +803,17 @@ static int parallelsNumOfDomains(virConnectPtr conn) { parallelsConnPtr privconn = conn-privateData; -int count; +virDomainObjListPtr domains = NULL; +int count = -1; -parallelsDriverLock(privconn); -count = virDomainObjListNumOfDomains(privconn-domains, 1); -parallelsDriverUnlock(privconn); +if (!(domains = parallelsGetDomains(privconn))) +goto cleanup; + +count = virDomainObjListNumOfDomains(domains, 1); +cleanup: +virDomainObjListDeinit(domains); +VIR_FREE(domains); return count; } @@ -812,14 +821,18 @@ static int parallelsListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { parallelsConnPtr privconn = conn-privateData; -int n; +virDomainObjListPtr domains = NULL; +int n = -1; + +if (!(domains = parallelsGetDomains(privconn))) +goto cleanup; -parallelsDriverLock(privconn); memset(names, 0, sizeof(*names) * maxnames); -n = virDomainObjListGetInactiveNames(privconn-domains, names, - maxnames); -parallelsDriverUnlock(privconn); +n = virDomainObjListGetInactiveNames(domains, names, maxnames); +cleanup: +virDomainObjListDeinit(domains); +VIR_FREE(domains); return n; } @@ -827,28 +840,39 @@ static int parallelsNumOfDefinedDomains(virConnectPtr conn) { parallelsConnPtr privconn = conn-privateData; -int count; +virDomainObjListPtr domains = NULL; +int count = -1; -parallelsDriverLock(privconn); -count = virDomainObjListNumOfDomains(privconn-domains, 0); -parallelsDriverUnlock(privconn); +if (!(domains = parallelsGetDomains(privconn))) +goto cleanup; +count = virDomainObjListNumOfDomains(domains, 0); + +cleanup: +virDomainObjListDeinit(domains); +VIR_FREE(domains); return count; } static int parallelsListAllDomains(virConnectPtr conn, -virDomainPtr **domains, +virDomainPtr **doms, unsigned int flags) { parallelsConnPtr privconn = conn-privateData; +virDomainObjListPtr domains = NULL; int ret = -1; virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); -parallelsDriverLock(privconn); -ret = virDomainList(conn, privconn-domains-objs, domains, flags); -parallelsDriverUnlock(privconn); +if (!(domains = parallelsGetDomains(privconn))) +goto cleanup; + +ret = virDomainList(conn, domains-objs, doms, flags); + +cleanup: +virDomainObjListDeinit(domains); +VIR_FREE(domains); return ret; } @@ -857,11 +881,13 @@ parallelsLookupDomainByID(virConnectPtr conn, int id) { parallelsConnPtr privconn = conn-privateData; virDomainPtr ret = NULL; -virDomainObjPtr dom; +virDomainObjPtr dom = NULL; +virDomainObjListPtr domains = NULL; -parallelsDriverLock(privconn); -dom = virDomainFindByID(privconn-domains, id); -parallelsDriverUnlock(privconn); +if (!(domains = parallelsGetDomains(privconn))) +goto cleanup; + +dom = virDomainFindByID(domains, id); if (dom == NULL) { virReportError(VIR_ERR_NO_DOMAIN, NULL); @@ -875,6 +901,8 @@ parallelsLookupDomainByID(virConnectPtr conn, int id) cleanup: if (dom) virDomainObjUnlock(dom); +virDomainObjListDeinit(domains); +VIR_FREE(domains); return ret; } @@ -883,15 +911,12 @@ parallelsLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { parallelsConnPtr privconn = conn-privateData; virDomainPtr ret = NULL; -virDomainObjPtr dom; +virDomainObjPtr dom = NULL; +char uuidstr[VIR_UUID_STRING_BUFLEN]; -parallelsDriverLock(privconn); -dom = virDomainFindByUUID(privconn-domains, uuid); -parallelsDriverUnlock(privconn); +virUUIDFormat(uuid, uuidstr); -if (dom == NULL) { -
Re: [libvirt] [PATCH 0/7 v4] Atomic API to list networks
Public git repo for easy reviewing (patches for all of the APIs are applied). git://github.com/OsierYang/libvirt.git On 2012年09月04日 23:55, Osier Yang wrote: v3 - v4: - Just rebase on top, and split the API from the big set Osier Yang (7): list: Define new API virConnectListAllNetworks list: Implement RPC calls for virConnectListAllNetworks list: Add helpers to list network objects list: Implement listAllNetworks for network driver list: Implement listAllNetworks for test driver list: Use virConnectListAllNetworks in virsh list: Expose virConnectListAllNetworks to Python binding daemon/remote.c | 55 + include/libvirt/libvirt.h.in | 20 ++ python/generator.py |1 + python/libvirt-override-api.xml |6 + python/libvirt-override-virConnect.py | 12 ++ python/libvirt-override.c | 48 + src/conf/network_conf.c | 91 + src/conf/network_conf.h | 22 ++ src/driver.h |5 + src/libvirt.c | 86 - src/libvirt_private.syms |1 + src/libvirt_public.syms |1 + src/network/bridge_driver.c | 17 ++ src/remote/remote_driver.c| 64 ++ src/remote/remote_protocol.x | 13 ++- src/remote_protocol-structs | 12 ++ src/test/test_driver.c| 17 ++ tools/virsh-network.c | 352 tools/virsh.pod | 12 +- 19 files changed, 743 insertions(+), 92 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Libguestfs] compile libguestfs 1.19.35
[The context for libvirt users is how to let people do 'make install' without having conflicts with installed copies of packages.] On Wed, Sep 05, 2012 at 07:39:37AM -0400, Whit Blauvelt wrote: On Wed, Sep 05, 2012 at 08:07:09AM +0100, Richard W.M. Jones wrote: It is possible to run 'make install', but we normally do not recommend you do that. To avoid conflicts between other packages, you should build a libguestfs package for your Linux distro, which is not so easy. Is there a goal at least that if a the group of programs including libvirt and qemu-kvm is built and installed from tar, that it should install with internal consistency regardless of underlying distro? All the distros necessarily lag development here. Sometimes new features are key to a particular deployment. Isn't it a useful goal to produce components such that, if make install is used with each of them, the result should be good? Is that the goal of these related projects? Now days, for instance, every distro does a good LAMP stack. But earlier in the development of that stack it was often best to compile it from source, due to versions lagging and distro maintainers making assumptions that weren't well honed. In the first few years of any new stack, shouldn't we expect that many sysadmins will be in the same position? It's best if there's some defined subset of programs whose make install options, by default, will produce a coherent stack. I'm not sure how special libvirt qemu are. You could try installing to a local prefix (I sometimes use ./configure --prefix=$HOME/gnu). Then you have to sort out the environment variables that need setting. I don't think libvirt or libguestfs or qemu document what environment variables should be set to run an internally consistent local copy of the entire libvirt/KVM stack from your home directory. libguestfs has the ./run script which can be modified. Anyone got any thoughts? I think it is a genuine concern. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Libguestfs] compile libguestfs 1.19.35
On Wed, Sep 05, 2012 at 12:46:26PM +0100, Richard W.M. Jones wrote: [The context for libvirt users is how to let people do 'make install' without having conflicts with installed copies of packages.] On Wed, Sep 05, 2012 at 07:39:37AM -0400, Whit Blauvelt wrote: On Wed, Sep 05, 2012 at 08:07:09AM +0100, Richard W.M. Jones wrote: It is possible to run 'make install', but we normally do not recommend you do that. To avoid conflicts between other packages, you should build a libguestfs package for your Linux distro, which is not so easy. Is there a goal at least that if a the group of programs including libvirt and qemu-kvm is built and installed from tar, that it should install with internal consistency regardless of underlying distro? All the distros necessarily lag development here. Sometimes new features are key to a particular deployment. Isn't it a useful goal to produce components such that, if make install is used with each of them, the result should be good? Is that the goal of these related projects? Now days, for instance, every distro does a good LAMP stack. But earlier in the development of that stack it was often best to compile it from source, due to versions lagging and distro maintainers making assumptions that weren't well honed. In the first few years of any new stack, shouldn't we expect that many sysadmins will be in the same position? It's best if there's some defined subset of programs whose make install options, by default, will produce a coherent stack. I'm not sure how special libvirt qemu are. You could try installing to a local prefix (I sometimes use ./configure --prefix=$HOME/gnu). Then you have to sort out the environment variables that need setting. I don't think libvirt or libguestfs or qemu document what environment variables should be set to run an internally consistent local copy of the entire libvirt/KVM stack from your home directory. libguestfs has the ./run script which can be modified. Anyone got any thoughts? I think it is a genuine concern. And DevStack is doing something similar for OpenStack developers ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Libguestfs] compile libguestfs 1.19.35
On Wed, Sep 05, 2012 at 12:46:26PM +0100, Richard W.M. Jones wrote: [The context for libvirt users is how to let people do 'make install' without having conflicts with installed copies of packages.] On Wed, Sep 05, 2012 at 07:39:37AM -0400, Whit Blauvelt wrote: On Wed, Sep 05, 2012 at 08:07:09AM +0100, Richard W.M. Jones wrote: It is possible to run 'make install', but we normally do not recommend you do that. To avoid conflicts between other packages, you should build a libguestfs package for your Linux distro, which is not so easy. Is there a goal at least that if a the group of programs including libvirt and qemu-kvm is built and installed from tar, that it should install with internal consistency regardless of underlying distro? All the distros necessarily lag development here. Sometimes new features are key to a particular deployment. Isn't it a useful goal to produce components such that, if make install is used with each of them, the result should be good? Is that the goal of these related projects? Now days, for instance, every distro does a good LAMP stack. But earlier in the development of that stack it was often best to compile it from source, due to versions lagging and distro maintainers making assumptions that weren't well honed. In the first few years of any new stack, shouldn't we expect that many sysadmins will be in the same position? It's best if there's some defined subset of programs whose make install options, by default, will produce a coherent stack. I'm not sure how special libvirt qemu are. You could try installing to a local prefix (I sometimes use ./configure --prefix=$HOME/gnu). Then you have to sort out the environment variables that need setting. I don't think libvirt or libguestfs or qemu document what environment variables should be set to run an internally consistent local copy of the entire libvirt/KVM stack from your home directory. libguestfs has the ./run script which can be modified. Anyone got any thoughts? I think it is a genuine concern. libvirt will search for QEMU binaries in $PATH. So if you install QEMU to a custom location, just make sure that location comes first in $PATH. WRT to libvirt.so - libvirtd, the socket path is based on the install prefix, so if you install libvirt to a non-standard location, you need to make sure you are using the matched libvirt.so and libvirtd. The easiest way todo this is just again make sure $PATH reflects your new install location, and set LD_LIBRARY_PATH so that libguestfs finds the custom libvirt.so too Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] (updated) additional parameters needed for dnsmasq
On 09/04/2012 11:12 AM, Eric Blake wrote: On 08/22/2012 11:47 AM, Eric Blake wrote: On 08/22/2012 11:39 AM, Eric Blake wrote: On 08/22/2012 10:59 AM, Gene Czarcinski wrote: As I said in a previous message, dnsmasq is forwarding a number of queries upstream that should not be done. There still remains an MX query for a plain name with no domain specified that will be forwarded is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not happen with no domain name and --local=// ... not a libvirt problem. ACK and pushed with the above tweak, and with adding you to AUTHORS (let us know if you prefer any other spelling or email address; the file is in UTF-8). Oh, and now that I've already pushed, I have a high-level question: what is the minimum version of 'dnsmasq' that supports the command-line syntax that this patch introduces? +--local=// --domain-needed --filterwin2k \ If older dnsmasq doesn't recognize --local=// or the new --domain-needed or --filterwin2k options, then we either need to make this code conditional based on probing 'dnsmasq --help' at startup, or else change the spec file to require a larger minimum version of dnsmasq (we already require 2.41 for other reasons). Just as I feared, we introduced a regression: https://bugzilla.redhat.com/show_bug.cgi?id=854137 Apparently, --filterwin2k disables features needed by Windows guests. Gene, what is the benefit vs. cost of adding this flag? I'm trying to figure out whether we need to expose it as something user-configurable, or whether we should just revert back to the pre-patch version that did not supply that option. I already had some second thoughts about --filterwin2k but you had pushed it. --filterwin2k should be removed. Unfortunately, as I said in another message, this --local= and --domain-needed are required but are not complete. dnsmasq will forward PTR queries which fall in the dnsmasq domain. I need to figure out how to create something of the form --local=/xxx.yyy.zzz.in-addr.arpa/. I have tested what the effect of this is and it works. Gene -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] [PATCH v2 1/6] add configure option --with-fuse for libvirt
On Wed, Sep 05, 2012 at 05:41:40PM +0800, Gao feng wrote: Hi Daniel Glauber 于 2012年07月31日 17:27, Daniel P. Berrange 写道: Hi Gao, I'm wondering if you are planning to attend the Linux Plumbers Conference in San Diego at the end of August ? Glauber is going to be giving a talk on precisely the subject of virtualizing /proc in containers which is exactly what your patch is looking at https://blueprints.launchpad.net/lpc/+spec/lpc2012-cont-proc I'll review your patches now, but I think I'd like to wait to hear what Glauber talks about at LPC before we try to merge this support in libvirt, so we have an broadly agreed long term strategy for /proc between all the interested userspace kernel guys. I did not attend the LPC,so can you tell me what's the situation of the /proc virtualization? I think maybe we should just apply this patchset first,and wait for somebody sending patches to implement /proc virtualization. So there were three main approaches discussed 1. FUSE based /proc + a real hidden /.proc. The FUSE /proc provides custom handling of various files like meminfo, otherwise forwards I/O requests through to the hidden /.proc files. This was the original proof of concept. 2. One FUSE filesystem for all containers + a real /proc. Bind mount files from the FUSE filesystem into the container's /proc. This is what Glauber has done. 3. One FUSE filesystem per container + a real /proc. Bind mount files from the FUSE filesystem into the container's /proc. This is what your patch is doing Options 2 3 have a clear a win over option 1 in efficiency terms, since they avoid doubling the I/O required for the majority of files. Glaubar thinks it is perferrable to have a single FUSE filesystem that has one sub-directory for each container. Then bind mount the appropriate sub dir into each container. I kinda like the way you have done things, having a private FUSE filesystem per container, for security reasons. By having the FUSE backend be part of the libvirt_lxc process we have strictly isolated each containers' environment. If we wanted a single shared FUSE for all containers, we'd need to have some single shared daemon to maintain it. This could not be libvirtd itself, since we need the containers their filesystems to continue to work when libvirtd itself is not running. We could introduce a separate libvirt_fused which provided a shared filesystem, but this still has the downside that any flaw in its impl could provide a way for one container to attack another container So in summary, I think your patches which add a private FUSE per container in libvirt_lxc appear to be the best option at this time. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv1 3/3] virsh: Update only changed scheduler tunables
When setting the cpu tunables in virsh you are able to update only one of them. Virsh while doing the update updated all of the tunables with old values except for the one changed. This is unfortunate as it: a) might overwrite some other change by a race condition (unprobable) b) fails with range checking as some of the old values saved might be out of range This patch changes the update procedure so that only the changed value is updated on the host. --- New in series. --- tools/virsh-domain.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index dcf2b8c..2abb457 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3390,7 +3390,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; virTypedParameterPtr params = NULL; int nparams = 0; -int update = 0; +int update = -1; int i, ret; bool ret_val = false; unsigned int flags = 0; @@ -3450,16 +3450,18 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (ret == 1) -update = 1; +update = i; } /* Update parameters refresh data */ -if (update) { +if (update = 0) { if (flags || current) -ret = virDomainSetSchedulerParametersFlags(dom, params, - nparams, flags); +ret = virDomainSetSchedulerParametersFlags(dom, + (params[update]), + 1, flags); else -ret = virDomainSetSchedulerParameters(dom, params, nparams); +ret = virDomainSetSchedulerParameters(dom, + (params[update]), 1); if (ret == -1) goto cleanup; -- 1.7.12 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/3] qemu: Add range checking for scheduler tunables when changed by API
The quota and period tunables for cpu scheduler accept only a certain range of values. When changing the live configuration invalid values get rejected. This check is not performed when changing persistent config. This patch adds a separate range check, that improves error messages when changing live config and adds the check for persistent config. This check is done only when using the API. It is still possible to specify invalid values in the XML. --- Diff to v1: - changed typecast to native long long specification --- src/qemu/qemu_driver.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4b8b751..8e8e00c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -106,6 +106,11 @@ #define QEMU_NB_TOTAL_CPU_STAT_PARAM 3 #define QEMU_NB_PER_CPU_STAT_PARAM 2 +#define QEMU_SCHED_MIN_PERIOD 1000LL +#define QEMU_SCHED_MAX_PERIOD 100LL +#define QEMU_SCHED_MIN_QUOTA 1000LL +#define QEMU_SCHED_MAX_QUOTA 18446744073709551LL + #if HAVE_LINUX_KVM_H # include linux/kvm.h #endif @@ -7790,6 +7795,15 @@ cleanup: return -1; } +#define SCHED_RANGE_CHECK(VAR, NAME, MIN, MAX) \ +if (((VAR) 0 (VAR) (MIN)) || (VAR) (MAX)) {\ +virReportError(VIR_ERR_INVALID_ARG, \ + _(value of '%s' is out of range [%lld, %lld]), \ + NAME, MIN, MAX); \ +rc = -1;\ +goto cleanup; \ +} + static int qemuSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, @@ -7876,6 +7890,9 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, vmdef-cputune.shares = value_ul; } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) { +SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, + QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); + if (flags VIR_DOMAIN_AFFECT_LIVE value_ul) { if ((rc = qemuSetVcpusBWLive(vm, group, value_ul, 0))) goto cleanup; @@ -7887,6 +7904,9 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, vmdef-cputune.period = params[i].value.ul; } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) { +SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, + QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); + if (flags VIR_DOMAIN_AFFECT_LIVE value_l) { if ((rc = qemuSetVcpusBWLive(vm, group, 0, value_l))) goto cleanup; @@ -7898,6 +7918,9 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, vmdef-cputune.quota = value_l; } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD)) { +SCHED_RANGE_CHECK(value_ul, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, + QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); + if (flags VIR_DOMAIN_AFFECT_LIVE value_ul) { if ((rc = qemuSetEmulatorBandwidthLive(vm, group, value_ul, 0))) goto cleanup; @@ -7909,6 +7932,9 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, vmdef-cputune.emulator_period = value_ul; } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA)) { +SCHED_RANGE_CHECK(value_l, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, + QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); + if (flags VIR_DOMAIN_AFFECT_LIVE value_l) { if ((rc = qemuSetEmulatorBandwidthLive(vm, group, 0, value_l))) goto cleanup; @@ -7944,6 +7970,7 @@ cleanup: qemuDriverUnlock(driver); return ret; } +#undef SCHED_RANGE_CHECK static int qemuSetSchedulerParameters(virDomainPtr dom, -- 1.7.12 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/3] Add range checking for scheduler tunables
This series adds range checking to cpu scheduler tunables when changed using the API. V2 contains a new patch that avoids changing all tunables from virsh when setting just a single one. Peter Krempa (3): qemu: clean up qemuSetSchedulerParametersFlags() qemu: Add range checking for scheduler tunables when changed by API virsh: Update only changed scheduler tunables src/qemu/qemu_driver.c | 96 ++ tools/virsh-domain.c | 14 2 files changed, 66 insertions(+), 44 deletions(-) -- 1.7.12 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/3] qemu: clean up qemuSetSchedulerParametersFlags()
This patch tries to clean the code up a little bit and shorten very long lines. The apparent semantic change from moving the condition before calling the setter function is a non-issue here as the setter function is a no-op when called with both arguments zero. --- Diff to v1: - enhanced commit message to explain apparent semantic change --- src/qemu/qemu_driver.c | 69 +++--- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b12d9bc..4b8b751 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7801,6 +7801,8 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; +unsigned long long value_ul; +long long value_l; int ret = -1; int rc; @@ -7857,74 +7859,65 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, for (i = 0; i nparams; i++) { virTypedParameterPtr param = params[i]; +value_ul = param-value.ul; +value_l = param-value.l; if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { if (flags VIR_DOMAIN_AFFECT_LIVE) { -rc = virCgroupSetCpuShares(group, params[i].value.ul); -if (rc != 0) { +if ((rc = virCgroupSetCpuShares(group, value_ul))) { virReportSystemError(-rc, %s, _(unable to set cpu shares tunable)); goto cleanup; } - -vm-def-cputune.shares = params[i].value.ul; +vm-def-cputune.shares = value_ul; } -if (flags VIR_DOMAIN_AFFECT_CONFIG) { -vmdef-cputune.shares = params[i].value.ul; -} +if (flags VIR_DOMAIN_AFFECT_CONFIG) +vmdef-cputune.shares = value_ul; + } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) { -if (flags VIR_DOMAIN_AFFECT_LIVE) { -rc = qemuSetVcpusBWLive(vm, group, params[i].value.ul, 0); -if (rc != 0) +if (flags VIR_DOMAIN_AFFECT_LIVE value_ul) { +if ((rc = qemuSetVcpusBWLive(vm, group, value_ul, 0))) goto cleanup; -if (params[i].value.ul) -vm-def-cputune.period = params[i].value.ul; +vm-def-cputune.period = value_ul; } -if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (flags VIR_DOMAIN_AFFECT_CONFIG) vmdef-cputune.period = params[i].value.ul; -} + } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) { -if (flags VIR_DOMAIN_AFFECT_LIVE) { -rc = qemuSetVcpusBWLive(vm, group, 0, params[i].value.l); -if (rc != 0) +if (flags VIR_DOMAIN_AFFECT_LIVE value_l) { +if ((rc = qemuSetVcpusBWLive(vm, group, 0, value_l))) goto cleanup; -if (params[i].value.l) -vm-def-cputune.quota = params[i].value.l; +vm-def-cputune.quota = value_l; } -if (flags VIR_DOMAIN_AFFECT_CONFIG) { -vmdef-cputune.quota = params[i].value.l; -} +if (flags VIR_DOMAIN_AFFECT_CONFIG) +vmdef-cputune.quota = value_l; + } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD)) { -if (flags VIR_DOMAIN_AFFECT_LIVE) { -rc = qemuSetEmulatorBandwidthLive(vm, group, params[i].value.ul, 0); -if (rc != 0) +if (flags VIR_DOMAIN_AFFECT_LIVE value_ul) { +if ((rc = qemuSetEmulatorBandwidthLive(vm, group, value_ul, 0))) goto cleanup; -if (params[i].value.ul) -vm-def-cputune.emulator_period = params[i].value.ul; +vm-def-cputune.emulator_period = value_ul; } -if (flags VIR_DOMAIN_AFFECT_CONFIG) { -vmdef-cputune.emulator_period = params[i].value.ul; -} +if (flags VIR_DOMAIN_AFFECT_CONFIG) +vmdef-cputune.emulator_period = value_ul; + } else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA)) { -if (flags VIR_DOMAIN_AFFECT_LIVE) { -rc = qemuSetEmulatorBandwidthLive(vm, group, 0, params[i].value.l); -if (rc != 0) +if (flags VIR_DOMAIN_AFFECT_LIVE value_l) { +if ((rc = qemuSetEmulatorBandwidthLive(vm, group, 0, value_l))) goto cleanup; -if (params[i].value.l) -vm-def-cputune.emulator_quota = params[i].value.l; +vm-def-cputune.emulator_quota = value_l; } -if (flags
Re: [libvirt] [RFC PATCH v1 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On Thu, Aug 23, 2012 at 16:31:51 +0530, Harsh Prateek Bora wrote: Qemu accepts gluster protocol as supported storage backend beside others. This patch allows users to specify disks on gluster backends like this: disk type='network' device='disk' driver name='qemu' type='raw'/ source protocol='gluster' name='volume/image' host name='example.org' port='6000' transport='socket'/ /source target dev='vda' bus='virtio'/ /disk Note: In the host element above, transport is an optional attribute. Valid transport types for a network based disk can be socket, unix or rdma. TODO: - Add support for IPv6 format based server addr - Support for transport types other than socket. Overall, this patch set looks fine. See my comments inline. Signed-off-by: Harsh Prateek Bora ha...@linux.vnet.ibm.com --- docs/schemas/domaincommon.rng | 8 +++ src/conf/domain_conf.c| 14 - src/conf/domain_conf.h| 3 +- src/qemu/qemu_command.c | 123 ++ 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 145caf7..30c0d8c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1029,6 +1029,7 @@ valuenbd/value valuerbd/value valuesheepdog/value +valuegluster/value /choice /attribute optional @@ -1042,6 +1043,13 @@ attribute name=port ref name=unsignedInt/ /attribute +attribute name=transport + choice +valuesocket/value +valueunix/value +valuerdma/value This could be a bit confusing as socket is too generic, after all unix is also a socket. Could we change the values tcp, unix, rdma or something similar depending on what socket was supposed to mean? + /choice +/attribute /element /zeroOrMore empty/ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 419088c..c89035e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -200,7 +200,8 @@ VIR_ENUM_IMPL(virDomainDiskErrorPolicy, VIR_DOMAIN_DISK_ERROR_POLICY_LAST, VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST, nbd, rbd, - sheepdog) + sheepdog, + gluster) We want to define a new enum for transport attribute in the same way we have it for protocol and use that enum instead of any free from string we parse from the XML or qemu command line. VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST, none, @@ -994,6 +995,7 @@ void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def) VIR_FREE(def-name); VIR_FREE(def-port); +VIR_FREE(def-transport); Then, there's no need to free it here. } void virDomainControllerDefFree(virDomainControllerDefPtr def) @@ -3489,6 +3491,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, } hosts[nhosts].name = NULL; hosts[nhosts].port = NULL; +hosts[nhosts].transport = NULL; nhosts++; hosts[nhosts - 1].name = virXMLPropString(child, name); @@ -3503,6 +3506,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, %s, _(missing port for host)); goto error; } +/* transport can be socket, unix, rdma, etc. */ +hosts[nhosts - 1].transport = virXMLPropString(child, transport); We would need to change this into calling the appropriate TypeFromString(). } child = child-next; } @@ -11479,8 +11484,13 @@ virDomainDiskDefFormat(virBufferPtr buf, for (i = 0; i def-nhosts; i++) { virBufferEscapeString(buf, host name='%s', def-hosts[i].name); -virBufferEscapeString(buf, port='%s'/\n, +virBufferEscapeString(buf, port='%s', def-hosts[i].port); +if (def-hosts[i].transport) { +virBufferEscapeString(buf, transport='%s', + def-hosts[i].transport); Call the appropriate TypeToString(def-hosts[i].transport) instead and align it with the first character
Re: [libvirt] [PATCH 1/6] list: Define new API virConnectListAllSecrets
On 09/05/12 08:28, Osier Yang wrote: This is to list the secret objects. No flags are supported include/libvirt/libvirt.h.in: Declare enum virConnectListAllSecretFlags and virConnectListAllSecrets. python/generator.py: Skip auto-generating src/driver.h: (virDrvConnectListAllSecrets) src/libvirt.c: Implement the public API src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in |3 ++ python/generator.py |1 + src/driver.h |5 src/libvirt.c| 50 ++ src/libvirt_public.syms |1 + 5 files changed, 60 insertions(+), 0 deletions(-) Looks fine. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v1 2/2] tests: Add tests for gluster protocol based network disks support
On Thu, Aug 23, 2012 at 16:31:52 +0530, Harsh Prateek Bora wrote: Signed-off-by: Harsh Prateek Bora ha...@linux.vnet.ibm.com --- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml| 33 ++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 37 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.xml Excellent. However, some changes may be needed if we end up with socket transport being renamed to something else. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] list: Implement RPC calls for virConnectListAllSecrets
On 09/05/12 08:28, Osier Yang wrote: The RPC generator doesn't support returning list of object yet, this patch do the work manually. * daemon/remote.c: Implemente the server side handler remoteDispatchConnectListAllSecrets. * src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllSecrets. * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_SECRETS and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 +++ src/remote/remote_driver.c | 64 ++ src/remote/remote_protocol.x | 13 - src/remote_protocol-structs | 12 4 files changed, 142 insertions(+), 1 deletions(-) ... diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 0dde9fc..afd3608 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2078,6 +2078,17 @@ struct remote_connect_list_all_nwfilters_ret { } filters; u_int ret; }; +struct remote_list_all_secrets_args { +intneed_results; +u_int flags; +}; +struct remote_list_all_secrets_ret { +struct { +u_int secrets_len; +remote_nonnull_network * secrets_val; This should be remote_nonnull_secret. +} secrets; +u_int ret; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2365,4 +2376,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES = 284, REMOTE_PROC_CONNECT_LIST_ALL_NODE_DEVICES = 285, REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, +REMOTE_PROC_CONNECT_LIST_ALL_SECRETS = 287, }; Otherwise looks good. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] NFS over RDMA small block DIRECT_IO bug
On 09/04/2012 03:04 PM, Myklebust, Trond wrote: On Tue, 2012-09-04 at 11:31 +0200, Andrew Holway wrote: Hello. # Avi Kivity avi(a)redhat recommended I copy kvm in on this. It would also seem relevent to libvirt. # I have a Centos 6.2 server and Centos 6.2 client. [root@store ~]# cat /etc/exports /dev/shm 10.149.0.0/16(rw,fsid=1,no_root_squash,insecure)(I have tried with non tempfs targets also) [root@node001 ~]# cat /etc/fstab store.ibnet:/dev/shm /mnt nfs rdma,port=2050,defaults 0 0 I wrote a little for loop one liner that dd'd the centos net install image to a file called 'hello' then checksummed that file. Each iteration uses a different block size. Non DIRECT_IO seems to work fine. DIRECT_IO with 512byte, 1K and 2K block sizes get corrupted. That is expected behaviour. DIRECT_IO over RDMA needs to be page aligned so that it can use the more efficient RDMA READ and RDMA WRITE memory semantics (instead of the SEND/RECEIVE channel semantics). Shouldn't subpage requests fail then? O_DIRECT block requests fail for subsector writes, instead of corrupting your data. Hopefully this is documented somewhere. -- error compiling committee.c: too many arguments to function -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] list: Implement listAllSecrets
On 09/05/12 08:28, Osier Yang wrote: Simply returns the object list. No filtering. src/secret/secret_driver.c: Implement listAllSecrets --- src/secret/secret_driver.c | 59 +++- 1 files changed, 58 insertions(+), 1 deletions(-) diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 7f92776..ed759ed 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -601,7 +601,6 @@ cleanup: return -1; } - Please don't include this hunk here. Two empty lines between functions are pretty common (this patch adds that too at the end of secretListAllSecrets(). static const char * secretUsageIDForDef(virSecretDefPtr def) { @@ -620,6 +619,63 @@ secretUsageIDForDef(virSecretDefPtr def) } } +static int +secretListAllSecrets(virConnectPtr conn, + virSecretPtr **secrets, + unsigned int flags) { +virSecretDriverStatePtr driver = conn-secretPrivateData; +virSecretPtr *tmp_secrets = NULL; +int nsecrets = 0; +int ret_nsecrets = 0; +virSecretPtr secret = NULL; +virSecretEntryPtr entry = NULL; +int i = 0; +int ret = -1; + +virCheckFlags(0, -1); + +secretDriverLock(driver); + +for (entry = driver-secrets; entry != NULL; entry = entry-next) +nsecrets++; + +if (!secrets) { +ret = nsecrets; +goto cleanup; +} + +if (VIR_ALLOC_N(tmp_secrets, nsecrets + 1) 0) { +virReportOOMError(); +goto cleanup; +} + +for (entry = driver-secrets; entry != NULL; entry = entry-next) { +if (!(secret = virGetSecret(conn, +entry-def-uuid, +entry-def-usage_type, +secretUsageIDForDef(entry-def +goto cleanup; +tmp_secrets[ret_nsecrets++] = secret; +} + +*secrets = tmp_secrets; +tmp_secrets = NULL; +ret = ret_nsecrets; + + cleanup: +secretDriverUnlock(driver); +if (tmp_secrets) { +for (i = 0; i ret_nsecrets; i ++) { +if (tmp_secrets[i]) +virSecretFree(tmp_secrets[i]); +} +} +VIR_FREE(tmp_secrets); + +return ret; +} + + static virSecretPtr secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { Otherwise looks OK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] list: Expose virConnectListAllSecrets to Python binding
On 09/05/12 08:28, Osier Yang wrote: The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: Implementation for listAllSecrets. python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml |6 python/libvirt-override-virConnect.py | 12 python/libvirt-override.c | 48 + 3 files changed, 66 insertions(+), 0 deletions(-) Looks good! Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] list: Expose virConnectListAllSecrets to Python binding
On 09/05/12 08:28, Osier Yang wrote: The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: Implementation for listAllSecrets. python/libvirt-override.c: Implementation for the wrapper. --- Huh, this commit message belongs to the previous patch. This is dealing with virsh. tools/virsh-secret.c | 182 ++ 1 files changed, 153 insertions(+), 29 deletions(-) diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index abcfdfe..c6193cc 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -290,6 +290,139 @@ cleanup: return ret; } +static int +vshSecretSorter(const void *a, const void *b) +{ +virSecretPtr *sa = (virSecretPtr *) a; +virSecretPtr *sb = (virSecretPtr *) b; +char uuid_sa[VIR_UUID_STRING_BUFLEN]; +char uuid_sb[VIR_UUID_STRING_BUFLEN]; + +if (*sa !*sb) +return -1; + +if (!*sa) +return *sb != NULL; + +virSecretGetUUIDString(*sa, uuid_sa); +virSecretGetUUIDString(*sb, uuid_sb); + +return vshStrcasecmp(uuid_sa, uuid_sb); +} + +struct vshSecretList { +virSecretPtr *secrets; +size_t nsecrets; +}; +typedef struct vshSecretList *vshSecretListPtr; + +static void +vshSecretListFree(vshSecretListPtr list) +{ +int i; + +if (list list-nsecrets) { +for (i = 0; i list-nsecrets; i++) { +if (list-secrets[i]) +virSecretFree(list-secrets[i]); +} +VIR_FREE(list-secrets); +} +VIR_FREE(list); +} + +static vshSecretListPtr +vshSecretListCollect(vshControl *ctl, + unsigned int flags) +{ +vshSecretListPtr list = vshMalloc(ctl, sizeof(*list)); +int i; +int ret; +virSecretPtr secret; +bool success = false; +size_t deleted = 0; +int nsecrets = 0; +char **uuids = NULL; + +/* try the list with flags support (0.10.2 and later) */ +if ((ret = virConnectListAllSecrets(ctl-conn, +list-secrets, +flags)) = 0) { +list-nsecrets = ret; +goto finished; +} + +/* check if the command is actually supported */ +if (last_error last_error-code == VIR_ERR_NO_SUPPORT) { +virFreeError(last_error); +last_error = NULL; I added a helper to reset libvirt errors: vshResetLibvirtError(); +goto fallback; +} + +/* there was an error during the call */ +vshError(ctl, %s, _(Failed to list node secrets)); +goto cleanup; + + +fallback: +/* fall back to old method (0.9.13 and older) */ +virResetLastError(); This needs to be changed to vshResetLibvirtError() as otherwise we would report a bad error in some cases. + +nsecrets = virConnectNumOfSecrets(ctl-conn); +if (nsecrets 0) { +vshError(ctl, %s, _(Failed to count secrets)); +goto cleanup; +} + +if (nsecrets == 0) +return list; + +uuids = vshMalloc(ctl, sizeof(char *) * nsecrets); + +nsecrets = virConnectListSecrets(ctl-conn, uuids, nsecrets); +if (nsecrets 0) { +vshError(ctl, %s, _(Failed to list secrets)); +goto cleanup; +} + +list-secrets = vshMalloc(ctl, sizeof(virSecretPtr) * (nsecrets)); +list-nsecrets = 0; + +/* get the secrets */ +for (i = 0; i nsecrets ; i++) { +if (!(secret = virSecretLookupByUUIDString(ctl-conn, uuids[i]))) +continue; +list-secrets[list-nsecrets++] = secret; +} + +/* truncate secrets that weren't found */ +deleted = nsecrets - list-nsecrets; + +finished: +/* sort the list */ +if (list-secrets list-nsecrets) +qsort(list-secrets, list-nsecrets, + sizeof(*list-secrets), vshSecretSorter); + +/* truncate the list for not found secret objects */ +if (deleted) +VIR_SHRINK_N(list-secrets, list-nsecrets, deleted); + +success = true; + +cleanup: +for (i = 0; i nsecrets; i++) +VIR_FREE(uuids[i]); +VIR_FREE(uuids); + +if (!success) { +vshSecretListFree(list); +list = NULL; +} + +return list; +} + /* * secret-list command */ @@ -302,56 +435,47 @@ static const vshCmdInfo info_secret_list[] = { static bool cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { -int maxuuids = 0, i; -char **uuids = NULL; - -maxuuids = virConnectNumOfSecrets(ctl-conn); -if (maxuuids 0) { -vshError(ctl, %s, _(Failed to list secrets)); -return false; -} -uuids = vshMalloc(ctl, sizeof(*uuids) * maxuuids); +int i; +vshSecretListPtr list = NULL; +bool ret = false; -maxuuids = virConnectListSecrets(ctl-conn, uuids, maxuuids); -if (maxuuids 0) { -vshError(ctl, %s, _(Failed to list secrets)); -VIR_FREE(uuids); +if
[libvirt] [PATCH] [trivial] Fix spelling mistake
provably - probably Signed-off-by: Philipp Hahn h...@univention.de --- cfg.mk |2 +- src/libvirt.c |2 +- tools/virsh.pod |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index 4cdfd1b..8fd9462 100644 --- a/cfg.mk +++ b/cfg.mk @@ -379,7 +379,7 @@ sc_prohibit_setuid: halt='use virSetUIDGID, not raw set*id' \ $(_sc_search_regexp) -# Use snprintf rather than s'printf, even if buffer is provably large enough, +# Use snprintf rather than s'printf, even if buffer is probably large enough, # since gnulib has more guarantees for snprintf portability sc_prohibit_sprintf: @prohibit='\[s]printf\' \ diff --git a/src/libvirt.c b/src/libvirt.c index b034ed6..041e4a6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17968,7 +17968,7 @@ error: * new hypervisor instance rather than reusing the existing hypervisor * (since this would terminate all connections to the domain, such as * such as VNC or Spice graphics) - this condition arises from active - * snapshots that are provably ABI incomaptible, as well as from + * snapshots that are probably ABI incomaptible, as well as from * inactive snapshots with a @flags request to start the domain after * the revert. * diff --git a/tools/virsh.pod b/tools/virsh.pod index a26c420..36b9593 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2685,7 +2685,7 @@ the case of reverting from a running domain to an active state where a new hypervisor has to be created rather than reusing the existing hypervisor, because it implies drawbacks such as breaking any existing VNC or Spice connections; this condition happens with an active -snapshot that uses a provably incompatible configuration, as well as +snapshot that uses a probably incompatible configuration, as well as with an inactive snapshot that is combined with the I--start or I--pause flag. -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] virsh: Remove unused vshNameSorter
On 09/05/12 08:28, Osier Yang wrote: vshNameSorter now is not used anymore. Remove it. --- tools/virsh.c |9 - tools/virsh.h |1 - 2 files changed, 0 insertions(+), 10 deletions(-) Yep, this function isn't used after all the changes. ACK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] list: Define new API virConnectListAllSecrets
On 09/05/2012 12:28 AM, Osier Yang wrote: This is to list the secret objects. No flags are supported Any reason we aren't allowing a filter on ephemeral=yes/no and private=yes/no from the secret XML? That is, I think you can usefully introduce two sets of filters based on those two binary properties of a secret. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6 v4] Atomic API to list secrets
The series looks fine and I'm giving ACK if you fix problems pointed out in the patches or other that may emerge. This API is a nice unification to the listing approach as there are no inactive secrets there's no race this would fix. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [trivial] Fix spelling mistake
On 09/05/2012 08:49 AM, Philipp Hahn wrote: provably - probably Signed-off-by: Philipp Hahn h...@univention.de --- cfg.mk |2 +- src/libvirt.c |2 +- tools/virsh.pod |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index 4cdfd1b..8fd9462 100644 --- a/cfg.mk +++ b/cfg.mk @@ -379,7 +379,7 @@ sc_prohibit_setuid: halt='use virSetUIDGID, not raw set*id' \ $(_sc_search_regexp) -# Use snprintf rather than s'printf, even if buffer is provably large enough, +# Use snprintf rather than s'printf, even if buffer is probably large enough, NACK. In this case, we meant 'provably', as in we can provide formal mathematical proof of the property. # since gnulib has more guarantees for snprintf portability sc_prohibit_sprintf: @prohibit='\[s]printf\' \ diff --git a/src/libvirt.c b/src/libvirt.c index b034ed6..041e4a6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17968,7 +17968,7 @@ error: * new hypervisor instance rather than reusing the existing hypervisor * (since this would terminate all connections to the domain, such as * such as VNC or Spice graphics) - this condition arises from active - * snapshots that are provably ABI incomaptible, as well as from + * snapshots that are probably ABI incomaptible, as well as from * inactive snapshots with a @flags request to start the domain after * the revert. Same NACK - we can prove whether a request is ABI incompatible. * diff --git a/tools/virsh.pod b/tools/virsh.pod index a26c420..36b9593 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2685,7 +2685,7 @@ the case of reverting from a running domain to an active state where a new hypervisor has to be created rather than reusing the existing hypervisor, because it implies drawbacks such as breaking any existing VNC or Spice connections; this condition happens with an active -snapshot that uses a provably incompatible configuration, as well as +snapshot that uses a probably incompatible configuration, as well as Same NACK - we can prove whether a configuration is compatible. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6 v4] Atomic API to list secrets
On 09/05/2012 09:03 AM, Peter Krempa wrote: The series looks fine and I'm giving ACK if you fix problems pointed out in the patches or other that may emerge. This API is a nice unification to the listing approach as there are no inactive secrets there's no race this would fix. Additionally, adding flags to filter on just ephemeral or private secrets will make this more powerful, but needs to be flowed through to more virsh flags. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v1 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On Wed, Sep 5, 2012 at 7:03 PM, Jiri Denemark jdene...@redhat.com wrote: @@ -1042,6 +1043,13 @@ attribute name=port ref name=unsignedInt/ /attribute +attribute name=transport + choice +valuesocket/value +valueunix/value +valuerdma/value This could be a bit confusing as socket is too generic, after all unix is also a socket. Could we change the values tcp, unix, rdma or something similar depending on what socket was supposed to mean? That is how gluster calls it and hence I am using the same in QEMU and the same is true here too. This is something for gluster developers to decide if they want to change socket to something more specific like tcp as you suggest. Regards, Bharata. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v1 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.
On 09/05/2012 09:08 AM, Bharata B Rao wrote: On Wed, Sep 5, 2012 at 7:03 PM, Jiri Denemark jdene...@redhat.com wrote: @@ -1042,6 +1043,13 @@ attribute name=port ref name=unsignedInt/ /attribute +attribute name=transport + choice +valuesocket/value +valueunix/value +valuerdma/value This could be a bit confusing as socket is too generic, after all unix is also a socket. Could we change the values tcp, unix, rdma or something similar depending on what socket was supposed to mean? That is how gluster calls it and hence I am using the same in QEMU and the same is true here too. This is something for gluster developers to decide if they want to change socket to something more specific like tcp as you suggest. Just because gluster calls it a confusing name does not mean we have to repeat the confusion in libvirt - it is feasible to have a mapping where we name it 'tcp' in the XML but map that to 'socket' in the command line that eventually reaches gluster. The question then becomes whether using sensible naming in libvirt, but no longer directly mapped to underlying gluster naming, will be the cause of its own set of headaches. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] 100+ misspellings
Hello, In case someone is interested in weeding out the inevitable false positives, here is a slightly filtered list of misspelled words in libvirt, as detected by the misspellings program from here: http://github.com/lyda/misspell-check Here's the output from this command: misspellings $(git ls-files)|grep -vE 'gnulib/|po/|ChangeLog-old' HACKING:456: particulary - particularly configure.ac:1359: presense - presence daemon/libvirtd.conf:254: upto - up to daemon/libvirtd.conf:270: upto - up to daemon/stream.c:692: upto - up to docs/api.html.in:67: garanteed - guaranteed docs/api.html.in:73: garanteed - guaranteed docs/apibuild.py:2476: unkown - unknown docs/apps.html.in:191: upto - up to docs/firewall.html.in:320: managable - manageable,manageably docs/formatdomain.html.in:423: ommitted - omitted docs/hacking.html.in:547: particulary - particularly docs/internals/locking.html.in:153: psuedo - pseudo docs/news.html.in:610: commited - committed docs/news.html.in:1195: dependancy - dependency docs/news.html.in:1459: doesnt - doesn't docs/news.html.in:1570: dependancy - dependency docs/news.html.in:1837: stange - strange docs/news.html.in:1838: dependancy - dependency docs/news.html.in:1892: sucessful - successful docs/news.html.in:1893: sucessful - successful docs/news.html.in:2032: sucessful - successful docs/news.html.in:2472: orginal - original docs/news.html.in:2628: statment - statement docs/news.html.in:2793: controling - controlling docs/news.html.in:3912: Seperate - Separate docs/news.html.in:5025: occurence - occurrence docs/news.html.in:5592: Explictly - Explicitly docs/news.html.in:6508: beeing - being docs/news.html.in:7158: adressing - addressing,dressing docs/news.html.in:7483: Supress - Suppress docs/news.html.in:7542: occurence - occurrence docs/news.html.in:7551: sucess - success docs/news.html.in:7825: neccessary - necessary docs/news.html.in:7902: Occurence - Occurrence docs/news.html.in:7902: occurences - occurrences docs/news.html.in:8472: wierd - weird docs/news.html.in:8535: accomodate - accommodate docs/news.html.in:8598: dependancy - dependency docs/news.html.in:9008: dependancy - dependency docs/news.html.in:9306: dependancy - dependency docs/news.html.in:9324: dependance - dependence docs/news.html.in:9520: extention - extension docs/news.html.in:9587: dependance - dependence docs/remote.html.in:359: agains - against docs/schemas/interface.rng:7: everytime - every time docs/schemas/interface.rng:175: grat - great examples/domain-events/events-python/event-test.py:129: occurr - occur examples/domain-events/events-python/event-test.py:172: upto - up to examples/python/dominfo.py:46: runing - running include/libvirt/libvirt.h.in:642: targetting - targeting include/libvirt/libvirt.h.in:2935: specfic - specific include/libvirt/libvirt.h.in:3207: upto - up to include/libvirt/libvirt.h.in:3639: occuring - occurring src/conf/domain_conf.c:4565: Unkown - Unknown src/conf/domain_conf.c:4933: Unkown - Unknown src/conf/domain_conf.c:6914: Wierd - Weird src/conf/nwfilter_conf.c:2804: definiton - definition src/internal.h:114: conciously - consciously src/libvirt.c:3083: transfered - transferred src/libvirt.c:4390: dependant - dependent src/libvirt.c:4441: dependant - dependent src/libvirt.c:11461: upto - up to src/libvirt.c:11544: upto - up to src/libvirt.c:14119: funtion - function src/libvirt.c:14542: dependant - dependent src/libvirt_internal.h:2: publically - publicly src/phyp/phyp_driver.c:2067: avaliable - available src/phyp/phyp_driver.c:2290: avaliable - available src/phyp/phyp_driver.c:2770: avaliable - available src/qemu/qemu_capabilities.c:659: existance - existence src/qemu/qemu_capabilities.c:1234: targetting - targeting src/qemu/qemu_conf.c:205: lenght - length src/qemu/qemu_driver.c:1235: calulate - calculate src/qemu/qemu_monitor_text.c:2406: everthing - everything src/qemu/qemu_process.c:2657: pased - passed src/rpc/virnetsaslcontext.c:153: Succesful - Successful src/rpc/virnettlscontext.c:377: Succesful - Successful src/storage/storage_backend_disk.c:536: boundry - boundary src/storage/storage_backend_disk.c:561: boundry - boundary src/storage/storage_backend_disk.c:571: boundry - boundary src/storage/storage_backend_disk.c:620: boundry - boundary src/storage/storage_backend_rbd.c:318: occured - occurred src/uml/uml_driver.c:260: upto - up to src/uml/uml_driver.c:1293: calulate - calculate src/util/cgroup.c:69: HIERACHY - Hierarchy src/util/cgroup.c:172: neccessarily - necessarily src/util/cgroup.c:592: HIERACHY - Hierarchy src/util/cgroup.c:1007: HIERACHY - Hierarchy src/util/cgroup.c:2029: existance - existence src/util/processinfo.c:80: upto - up to src/util/processinfo.c:145: upto - up to src/util/virpidfile.c:369: agin - again src/util/virpidfile.c:378: agin - again src/util/virsocketaddr.c:421: recieve - receive src/util/virsocketaddr.c:451: recieve - receive src/util/virtime.c:105: THRU - Through src/util/virtime.c:162: THRU - Through src/util/virtime.c:163: THRU -
Re: [libvirt] [PATCH] (updated) additional parameters needed for dnsmasq
On 09/05/2012 07:55 AM, Gene Czarcinski wrote: On 09/04/2012 11:12 AM, Eric Blake wrote: On 08/22/2012 11:47 AM, Eric Blake wrote: On 08/22/2012 11:39 AM, Eric Blake wrote: On 08/22/2012 10:59 AM, Gene Czarcinski wrote: As I said in a previous message, dnsmasq is forwarding a number of queries upstream that should not be done. There still remains an MX query for a plain name with no domain specified that will be forwarded is dnsmasq has --domain=xxx --local=/xxx/ specified. This does not happen with no domain name and --local=// ... not a libvirt problem. ACK and pushed with the above tweak, and with adding you to AUTHORS (let us know if you prefer any other spelling or email address; the file is in UTF-8). Oh, and now that I've already pushed, I have a high-level question: what is the minimum version of 'dnsmasq' that supports the command-line syntax that this patch introduces? +--local=// --domain-needed --filterwin2k \ If older dnsmasq doesn't recognize --local=// or the new --domain-needed or --filterwin2k options, then we either need to make this code conditional based on probing 'dnsmasq --help' at startup, or else change the spec file to require a larger minimum version of dnsmasq (we already require 2.41 for other reasons). Just as I feared, we introduced a regression: https://bugzilla.redhat.com/show_bug.cgi?id=854137 Apparently, --filterwin2k disables features needed by Windows guests. Gene, what is the benefit vs. cost of adding this flag? I'm trying to figure out whether we need to expose it as something user-configurable, or whether we should just revert back to the pre-patch version that did not supply that option. I already had some second thoughts about --filterwin2k but you had pushed it. --filterwin2k should be removed. Yes, as rare as dialup lines are these days, I think it's highly unlikely that anyone running a virt host will be connected to the rest of the network in a way which will require bringing up a dialup network connection in order to send a packet to a domain controller. So, I don't think we should clutter the XML with such a specific option that will in all likelyhood never be used. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/10] list: Implement listAllStoragePools for test driver
On 09/04/2012 09:16 AM, Osier Yang wrote: src/test/test_driver.c: Implement listAllStoragePools --- src/test/test_driver.c | 17 + 1 files changed, 17 insertions(+), 0 deletions(-) @@ -5662,6 +5678,7 @@ static virStorageDriver testStorageDriver = { .listPools = testStorageListPools, /* 0.5.0 */ .numOfDefinedPools = testStorageNumDefinedPools, /* 0.5.0 */ .listDefinedPools = testStorageListDefinedPools, /* 0.5.0 */ +.listAllPools = testStorageListAllPools, /* 0.10.0 */ 0.10.2. ACK with that fix. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/10] virsh: Fix the wrong doc for pool-list
On 09/04/2012 09:16 AM, Osier Yang wrote: The storage pool's management doesn't relate with a domain, it probably was an intention, but not achieved yet. And the fact is only active pools are listed by default. --- tools/virsh.pod |9 - 1 files changed, 4 insertions(+), 5 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] list: Add helper to convert strings separated by ', ' to array
On 09/04/2012 09:16 AM, Osier Yang wrote: tools/virsh.c: New helper function vshStringToArray. tools/virsh.h: Declare vshStringToArray. tools/virsh-domain.c: use the helper in cmdUndefine. --- tools/virsh-domain.c | 19 ++- tools/virsh.c| 44 tools/virsh.h|1 + 3 files changed, 47 insertions(+), 17 deletions(-) ACK. This is a nice refactor for later use. But down the road, do we want to borrow a leaf from qemu's book, and allow for users to input ',,' for a literal comma that does not separate options? If so, then we should merge in the double comma handling from vshParseSnapshotDiskspec (virsh-snapshot.c) into this function, and have that function also refactored to use this helper. Without something like that, users cannot input a literal comma when listing comma-separated arguments. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/10] list: Change MATCH for common use in virsh
On 09/04/2012 09:16 AM, Osier Yang wrote: Move definition of MATCH from virsh-domain-monitor.c into virsh.h for further use. --- tools/virsh-domain-monitor.c |2 -- tools/virsh.h|2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) Not quite right. +++ b/tools/virsh.h @@ -47,6 +47,8 @@ # define GETTIMEOFDAY(T) gettimeofday(T, NULL) +# define MATCH(FLAG) (flags (FLAG)) If you're going to make the macro reusable, I think it would be better to put it in the VSH_ namespace, and update all existing callers. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10 v5] list: Use virConnectListAllStoragePools in virsh
On 09/05/2012 12:36 AM, Osier Yang wrote: tools/virsh-pool.c: * vshStoragePoolSorter to sort the pool list by pool name. * struct vshStoragePoolList to present the pool list, pool info is collected by list-poolinfo if 'details' is specified by user. * vshStoragePoolListFree to free the pool list * vshStoragePoolListCollect to collect the pool list, new API virStorageListAllPools is tried first, if it's not supported, fall back to older APIs. * New options --persistent, --transient, --autostart, --no-autostart and --type for pool-list. --persistent or --transient is to filter the returned pool list by whether the pool is persistent or not. --autostart or --no-autostart is to filter the returned pool list by whether the pool is autostarting or not. --type is to filter the pools by pool types. E.g. % virsh pool-list --all --persistent --type dir,disk tools/virsh.pod: * Add documentations for the new options. --- If you rename things to VSH_MATCH in 8/10, then this patch will be impacted. +++ b/tools/virsh-pool.c @@ -36,6 +36,7 @@ #include memory.h #include util.h #include xml.h +#include conf/storage_conf.h I'm not sure if virsh is supposed to be able to use conf/*.h files; you're not the first offender, but the more we do this, the more we are admitting that our public API is insufficient. I'm wondering if we should move the filter group constants into libvirt.h, and make them part of the public API... Regardless of what we decide about the above, though, I think it would be independent cleanup patches and doesn't affect this patch. +static vshStoragePoolListPtr +vshStoragePoolListCollect(vshControl *ctl, + unsigned int flags) +{ +vshStoragePoolListPtr list = vshMalloc(ctl, sizeof(*list)); +int i; +int ret; +char **names = NULL; +virStoragePoolPtr pool; +bool success = false; +size_t deleted = 0; +int persistent; +int autostart; +int nActivePools = 0; +int nInactivePools = 0; +int nAllPools = 0; + +/* try the list with flags support (0.10.0 and later) */ 0.10.2 + +if (last_error last_error-code == VIR_ERR_INVALID_ARG) { Why two spaces after ==? + +fallback: +/* fall back to old method (0.9.13 and older) */ 0.10.1 @@ -563,6 +790,11 @@ static const vshCmdInfo info_pool_list[] = { static const vshCmdOptDef opts_pool_list[] = { {inactive, VSH_OT_BOOL, 0, N_(list inactive pools)}, {all, VSH_OT_BOOL, 0, N_(list inactive active pools)}, +{transient, VSH_OT_BOOL, 0, N_(list transient pools)}, +{persistent, VSH_OT_BOOL, 0, N_(list persistent pools)}, +{autostart, VSH_OT_BOOL, 0, N_(list pools with autostart enabled)}, +{no-autostart, VSH_OT_BOOL, 0, N_(list pools with autostart disabled)}, +{type, VSH_OT_STRING, 0, N_(only list pool of specified type(s) (if supported))}, Missing a plural, and the parenthetical '(if supported)' doesn't add any value. Maybe: N_(filter pools by type (multiple types separated by commas)) +if (type) { +int poolType = -1; +char **poolTypes = NULL; +int npoolTypes = 0; + +npoolTypes = vshStringToArray((char *)type, poolTypes); + +for (i = 0; i npoolTypes; i++) { +if ((poolType = virStoragePoolTypeFromString(poolTypes[i])) 0) { +vshError(ctl, %s, _(Invalid pool type)); Hmm. What happens if we add new pool types in the future? If libvirt 0.10.3 supports the new pool type 'foo', but I am connecting with virsh 0.10.2 as the client, then I am unable to specify the pool type, even though it is valid to the server. But I don't have any good suggestions on how to make the full list of supported pool types available, and which bits they map to, unless we do something like add a new API to return a list of supported pool types and then have this virsh function consult that list instead of hard-coding the list at the time of the libvirt release when virsh was compiled. Probably not worth the effort. +case VIR_STORAGE_POOL_RBD: +flags |= VIR_CONNECT_LIST_STORAGE_POOLS_RBD; +break; +default: +break; If we hit the default case, that means that conf/storage_conf.h has added new pool types but we forgot to update this case statement to cope with them. I think you want to raise an error here instead of doing silent fallthrough, so that we can diagnose such an issue. +++ b/tools/virsh.pod @@ -2185,13 +2185,33 @@ variables, and defaults to Cvi. Returns basic information about the Ipool object. -=item Bpool-list [I--inactive | I--all] [I--details] +=item Bpool-list [I--inactive] [I--all] + [I--persistent] [I--transient] + [I--autostart] [I--no-autostart] + [[I--details] [type] List pool objects known to
Re: [libvirt] [PATCH 10/10] python: Expose virStorageListAllStoragePools to python binding
On 09/04/2012 09:16 AM, Osier Yang wrote: The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. Given the number of times we've made comments like this in the overall series, I can't help but wonder if it is worth teaching the generators (RPC and python bindings) about wrapping the listall APIs. But that's icing on top. ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V4] implement offline migration
On Wed, Sep 05, 2012 at 09:48:22AM +0100, Daniel P. Berrange wrote: On Wed, Sep 05, 2012 at 08:51:05AM +0800, liguang wrote: 在 2012-09-04二的 12:12 +0100,Daniel P. Berrange写道: On Mon, Sep 03, 2012 at 02:23:24PM +0800, liguang wrote: allow migration even domain isn't active by inserting some stubs to tunnel migration path. Signed-off-by: liguang lig.f...@cn.fujitsu.com --- src/qemu/qemu_driver.c|2 +- src/qemu/qemu_migration.c | 181 +++-- src/qemu/qemu_migration.h |3 +- 3 files changed, 178 insertions(+), 8 deletions(-) I really don't like the general design of this patch, even ignoring all the code bugs. I think this entire patch is really just a solution in search of a problem. Offline migration is already possible with existing libvirt APIs: domsrc = virDomainLookupByName(connsrc, someguest); xml = virDomainGetXMLDesc(domsrc); domdst virDomainDefine(conndst, xml); Um, maybe you mean offline migration is just redefinition of domain at target side, but what about disk images the domain used without sharing files between source and target, do we have to take a look at this case? Which can also be done already virStorageVolDownload + virStorageVolUpload which lets the app choose exactly which disks images they wish to copy, rather than assuming all of them should be copied. Daniel, One use case for the VolDownload/Upload APIs that I've been wondering about for a while is a data center with good connectivity (say 10Gbps) between hosts but relatively constrained bandwidth (say 100Mbps or WAN) on the management network. What's the best approach to copying large volumes between hosts in that case? Dave Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10 v5] list: Use virConnectListAllStoragePools in virsh
On 09/05/2012 10:52 AM, Eric Blake wrote: +for (i = 0; i npoolTypes; i++) { +if ((poolType = virStoragePoolTypeFromString(poolTypes[i])) 0) { +vshError(ctl, %s, _(Invalid pool type)); Hmm. What happens if we add new pool types in the future? If libvirt 0.10.3 supports the new pool type 'foo', but I am connecting with virsh 0.10.2 as the client, then I am unable to specify the pool type, even though it is valid to the server. But I don't have any good suggestions on how to make the full list of supported pool types available, and which bits they map to, unless we do something like add a new API to return a list of supported pool types and then have this virsh function consult that list instead of hard-coding the list at the time of the libvirt release when virsh was compiled. Probably not worth the effort. Or maybe instead of a new API, we reuse existing capability XML to expose the full list of supported pool types as a capability. I know I'd also like capability XML to show the supported names of domxml-{to,from}-native transformations possible for a given driver. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V4] implement offline migration
On 09/05/2012 02:48 AM, Daniel P. Berrange wrote: I really don't like the general design of this patch, even ignoring all the code bugs. I think this entire patch is really just a solution in search of a problem. Offline migration is already possible with existing libvirt APIs: I agree that the existing patches are making too many assumptions and not honoring flags correctly; but I'm still not sure why the user must decompose offline migration into a sequence of calls... domsrc = virDomainLookupByName(connsrc, someguest); xml = virDomainGetXMLDesc(domsrc); domdst virDomainDefine(conndst, xml); Um, maybe you mean offline migration is just redefinition of domain at target side, but what about disk images the domain used without sharing files between source and target, do we have to take a look at this case? Which can also be done already virStorageVolDownload + virStorageVolUpload ...when a single virMigrate API could do the same decomposition as syntactic sugar, if the patch were cleaned up to actually obey flags. That is, why must virMigrate be a live-only operation, forcing virt-manager and all other wrappers to re-implement the same giant sequence of API calls for offline migration? -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't pin all the cpus
On 04.09.2012 16:23, Martin Kletzander wrote: This is another fix for the emulator-pin series. When going through the cputune pinning settings, the current code is trying to pin all the CPUs, even when not all of them are specified. This causes error in the subsequent function which, of course, cannot find the cpu to pin. Since it's enough to pass the correct VCPU ID to the function, the fix is trivial. --- src/qemu/qemu_cgroup.c | 25 + 1 file changed, 17 insertions(+), 8 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: use re-entrant functions in virsh
On 09/05/2012 12:52 AM, Martin Kletzander wrote: On 09/05/2012 01:36 AM, Eric Blake wrote: Today's patches pointed out that virsh was still using localtime(), which is not thread-safe, even though virsh is definitely multi-threaded. * cfg.mk (exclude_file_name_regexp--sc_prohibit_nonreentrant): Tighten the rule. * tools/virsh.c (vshOutputLogFile): Avoid localtime. (vshEditWriteToTempFile, vshEditReadBackFile, cmdCd, cmdPwd) (vshCloseLogFile): Avoid strerror. * tools/console.c (vshMakeStdinRaw): Likewise. * tools/virsh-domain.c (vshGenFileName): Fix spacing in previous patch. --- cfg.mk | 2 +- tools/console.c | 11 +++ tools/virsh-domain.c | 2 +- tools/virsh.c| 40 4 files changed, 33 insertions(+), 22 deletions(-) ACK, Thanks; pushed, after tweaking the commit message to call out a commit id (as it is no longer today's patches, but yesterday). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10 v5] list: Use virConnectListAllStoragePools in virsh
On 09/05/2012 12:52 PM, Eric Blake wrote: On 09/05/2012 12:36 AM, Osier Yang wrote: +++ b/tools/virsh-pool.c @@ -36,6 +36,7 @@ #include memory.h #include util.h #include xml.h +#include conf/storage_conf.h I'm not sure if virsh is supposed to be able to use conf/*.h files; you're not the first offender, but the more we do this, the more we are admitting that our public API is insufficient. I'm wondering if we should move the filter group constants into libvirt.h, and make them part of the public API... Yes. (or whatever it takes to not use non-public .h files in virsh). virsh should only use the public libvirt API; if it needs something that's private to libvirt, either that piece of code should be rewritten, or the public API should be enhanced. (But, as you say, Osier isn't the first offender, so it's okay to let this temporarily slip by). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't pin all the cpus
On 09/05/2012 07:11 PM, Michal Privoznik wrote: On 04.09.2012 16:23, Martin Kletzander wrote: This is another fix for the emulator-pin series. When going through the cputune pinning settings, the current code is trying to pin all the CPUs, even when not all of them are specified. This causes error in the subsequent function which, of course, cannot find the cpu to pin. Since it's enough to pass the correct VCPU ID to the function, the fix is trivial. --- src/qemu/qemu_cgroup.c | 25 + 1 file changed, 17 insertions(+), 8 deletions(-) ACK Michal Thanks, pushed. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix adding ports to OVS bridges without VLAN tags
On 09/04/2012 05:03 PM, Kyle Mestery (kmestery) wrote: On Aug 31, 2012, at 9:09 AM, Daniel Veillard wrote: On Fri, Aug 31, 2012 at 01:32:34PM +, Kyle Mestery (kmestery) wrote: On Aug 30, 2012, at 10:23 PM, Daniel Veillard wrote: On Thu, Aug 30, 2012 at 04:38:06PM -0400, Kyle Mestery wrote: [...] Still there is something which looks wrong, if we don't have a profileID why do we end up with instead of NULL ? I'm seeing various tests for profileID[0] over conf/*.c and util/*.c , and that sounds wrong to me. if there is no data, store NULL ! Then test for profileID instead of profileID[0]. Then there is no risk of a crash because abscence of data led to NULL instead of an empty string, the code is more resilient ! I expect a followup patch cleaning this up, but after 0.10.1 ... thanks ! Thanks Daniel, I'll work on the followup patch today. No hurry, because I just rolled 0.10.1 out so that won't make it (and it's not urgent). Giving 0.10.1 a try would be nice too, thanks ! Daniel Hi Daniel: Picking this back up. struct _virNetDevVPortProfile contains the following: /* this member is used when virtPortType == 802.1Qbh|openvswitch */ /* this is a null-terminated character string */ char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; To address your comments around checking for profileID[0], we could make profileID here a pointer, and allocate it when we allocate a struct _virNetDevVPortProfile object. But to me, having a fixed length string in this structure doesn't seem wrong. Copying Laine here as well for his comments, but I'm inclined to leave things as they I'm not sure why all the strings in virNetDevVPortProfile were made fixed length, but that's what we ended up with (possibly because the initial fields were all fixed length, and the person adding things that could be variable length wanted to avoid potentially forgetting to free a subordinate memory chunk when freeing a virNetDevVPortProfile? Or maybe netlink really does limit the maximum length of the profileID?). As a matter of fact, that's why the xxx_specified booleans are there - because there is no implicit specified flag due to a pointer being NULL/non-NULL. I'm with Daniel in preferring the string attributes of virNetDevVPortProfile to each be a pointer to a separate chunk of memory (and we should maybe do that in a cleanup patch), but in this case the new code added by Kyle is correct. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] VM Migration with IEEE802.1Qbh and IEEE802.1Qbg
On 09/04/2012 11:14 AM, Laine Stump wrote: On 09/04/2012 04:29 AM, Martin Kletzander wrote: There should be one more option in case the virtualport is specified in the guest's interface. According to the docs [2], when no type= is specified for the virtualport element, this should be auto-completed on the domain startup, but that's most probably not applicable for your situation (migration). That is only the case for 1) very new libvirt (0.10.0+) and 2) interface type='network'. It's purpose is to allow specifying, for example, both an openvswitch interfaceid and an 802.11Qbh profileid at the same time (for cases when the guest doesn't know whether the network it connects to will be openvswitch or 802.1Qbh, and wants to specify the appropriate setting for both possibilities). I picked a bad example (Open vSwitch also uses profileid). Think instead of the case where you didn't know if the host was going to be openvswitch (which uses interfaceid) or 802.1Qbg (which uses instanceid). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] build: don't fail when xsltproc is missing
On a machine without xsltproc, the build failed with: Scripting search.php /usr/local/bin/bash: line 1: search.php.tmp: No such file or directory rm: ./search.php: No such file or directory Regression introduced in commit 28183590. * docs/Makefile.am (%.php): Skip in the same conditions when the .tmp file is skipped. --- docs/Makefile.am | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index fad8979..91c3d33 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -186,10 +186,11 @@ internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in || { rm $@ exit 1; }; fi %.php: %.php.tmp %.php.code.in - @echo Scripting $@; \ - sed -e '/a id=php_placeholder\/a/r '$(srcdir)/$@.code.in \ - -e /php_placeholder/d $@.tmp $(srcdir)/$@ \ - || { rm $(srcdir)/$@ exit 1; } + @if [ -x $(XSLTPROC) ] ; then \ + echo Scripting $@; \ + sed -e '/a id=php_placeholder\/a/r '$(srcdir)/$@.code.in \ + -e /php_placeholder/d $@.tmp $(srcdir)/$@ \ + || { rm $(srcdir)/$@ exit 1; }; fi html/index.html: libvirt-api.xml newapi.xsl page.xsl sitemap.html.in $(AM_V_GEN)if [ -x $(XSLTPROC) ] ; then \ -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix 'make' issues on OpenBSD.
Pushing both under the build-breaker rule. 'make check' still fails some gnulib tests on OpenBSD, but that's not quite as severe. Eric Blake (2): build: avoid warnings from gcc 4.2.1 build: don't fail when xsltproc is missing docs/Makefile.am| 9 + m4/virt-compile-warnings.m4 | 8 +++- src/internal.h | 6 -- 3 files changed, 16 insertions(+), 7 deletions(-) -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] build: avoid warnings from gcc 4.2.1
OpenBSD ships with gcc 4.2.1, which annoyingly treats all format strings as though they were also attribute((nonnull)). The two concepts are orthogonal, though, as evidenced by the number of spurious warnings it generates on uses where we know that virReportError specifically handles NULL instead of a format string; worse, since we now force -Werror on git builds, it prevents development builds on OpenBSD. I hate to do this, as it disables ALL format checking on older gcc, and therefore misses out on some useful checks (code that happened to compile on Linux may still have type mismatches when compiled on other platforms, as evidenced by the number of times I have fixed formatting mismatches for uid_t as found by warnings on Cygwin), but I don't see any other way to keep -Werror alive and still compile on OpenBSD. A more invasive change would be to make virReportError() mark its format attribute as nonnull, and fix (a lot of) fallout; we may end up doing that anyways as part of danpb's error refactoring improvements, but not today. * src/internal.h (ATTRIBUTE_FMT_PRINTF): Use preferred spellings. * m4/virt-compile-warnings.m4 (-Wformat): Disable on older gcc. --- m4/virt-compile-warnings.m4 | 8 +++- src/internal.h | 6 -- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 26f6134..c3ff962 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -110,8 +110,14 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ gl_WARN_ADD([-Wjump-misses-init]) # GNULIB turns on -Wformat=2 which implies -Wformat-nonliteral, -# so we need to manually re-exclude it. +# so we need to manually re-exclude it. Also, older gcc 4.2 +# added an implied ATTRIBUTE_NONNULL on any parameter marked +# ATTRIBUTE_FMT_PRINT, which causes -Wformat failure on our +# intentional use of virReportError(code, NULL). gl_WARN_ADD([-Wno-format-nonliteral]) +if test $lv_cv_gcc_pragma_push_works = no; then + gl_WARN_ADD([-Wno-format]) +fi # This should be 256 really. Currently we're down to 4096, # but using 1024 bytes sized buffers (mostly for virStrerror) diff --git a/src/internal.h b/src/internal.h index 832a931..02fdb8d 100644 --- a/src/internal.h +++ b/src/internal.h @@ -151,9 +151,11 @@ */ # ifndef ATTRIBUTE_FMT_PRINTF # if __GNUC_PREREQ (4, 4) -#define ATTRIBUTE_FMT_PRINTF(fmtpos,argpos) __attribute__((__format__ (gnu_printf, fmtpos,argpos))) +#define ATTRIBUTE_FMT_PRINTF(fmtpos,argpos) \ +__attribute__((__format__ (__gnu_printf__, fmtpos, argpos))) # else -#define ATTRIBUTE_FMT_PRINTF(fmtpos,argpos) __attribute__((__format__ (printf, fmtpos,argpos))) +#define ATTRIBUTE_FMT_PRINTF(fmtpos,argpos) \ +__attribute__((__format__ (__printf__, fmtpos, argpos))) # endif # endif -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] network: prevent infinite hang if ovs-vswitchd isn't running
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=852984 If a network or interface is configured to use Open vSwitch, but ovs-vswitchd (the Open vSwitch database service) isn't running, the ovs-vsctl add-port/del-port commands will hang indefinitely rather than returning an error. There is a --nowait option, but that appears to have no effect on add-port and del-port commands, so instead we add a --timeout=5 to the commands - they will retry for up to 5 seconds, then fail if there is no response. --- I'm not sure if 5 seconds is a reasonable timeout for this, but it seems to work okay for me (in a very lightly loaded environment, but a successful return is always effectively instantaneous...). Unfortunately there doesn't seem to be another way to bound the wait time of ovs-vsctl. :-( src/util/virnetdevopenvswitch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 764f478..97b9d52 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -105,7 +105,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cmd = virCommandNew(OVSVSCTL); -virCommandAddArgList(cmd, --, --may-exist, add-port, +virCommandAddArgList(cmd, --timeout=5, --, --may-exist, add-port, brname, ifname, NULL); if (virBufferUse(buf) != 0) @@ -166,7 +166,7 @@ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const ch virCommandPtr cmd = NULL; cmd = virCommandNew(OVSVSCTL); -virCommandAddArgList(cmd, --, --if-exists, del-port, ifname, NULL); +virCommandAddArgList(cmd, --timeout=5, --, --if-exists, del-port, ifname, NULL); if (virCommandRun(cmd, NULL) 0) { virReportSystemError(VIR_ERR_INTERNAL_ERROR, -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: prevent infinite hang if ovs-vswitchd isn't running
On Sep 5, 2012, at 1:24 PM, Laine Stump wrote: This fixes https://bugzilla.redhat.com/show_bug.cgi?id=852984 If a network or interface is configured to use Open vSwitch, but ovs-vswitchd (the Open vSwitch database service) isn't running, the ovs-vsctl add-port/del-port commands will hang indefinitely rather than returning an error. There is a --nowait option, but that appears to have no effect on add-port and del-port commands, so instead we add a --timeout=5 to the commands - they will retry for up to 5 seconds, then fail if there is no response. This looks good. I agree with your comments here around the 5 second timeout not being ideal, but being workable. Acked-by: Kyle Mestery kmest...@cisco.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/2] build: avoid check-symfile on non-Linux
I tested both OpenBSD and cygwin; both failed 'make check' with: GENcheck-symfile Can't return outside a subroutine at ./check-symfile.pl line 13. Perl requires 'exit 77' instead of 'return 77' in that context, but even with that tweak, the build still fails, since the exit code of 77 is only special to explicit TESTS=foo listings, and not to make-only dependency rules where we are not going through automake's test framework. * src/check-symfile.pl: Kill bogus platform check... * src/Makefile.am (check-symfile): ...and replace with an automake conditional. --- Another push under the build-breaker rule. src/Makefile.am | 6 +- src/check-symfile.pl | 5 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 39adeac..9f27fcf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -312,10 +312,14 @@ PDWTAGS = \ fi # .libs/libvirt.so is built by libtool as a side-effect of the Makefile -# rule for libvirt.la +# rule for libvirt.la. However, checking symbols relies on Linux ELF layout +if WITH_LINUX check-symfile: libvirt.syms libvirt.la $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \ .libs/libvirt.so +else +check-symfile: +endif PROTOCOL_STRUCTS = \ $(srcdir)/remote_protocol-structs \ diff --git a/src/check-symfile.pl b/src/check-symfile.pl index 5454f45..454fed3 100755 --- a/src/check-symfile.pl +++ b/src/check-symfile.pl @@ -8,11 +8,6 @@ my @elflibs = @ARGV; my @wantsyms; my %gotsyms; -# Skip on non-linux -if ($^O ne linux) { -return 77; # Automake's skip code -} - open SYMFILE, $symfile or die cannot read $symfile: $!; while (SYMFILE) { -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: prevent infinite hang if ovs-vswitchd isn't running
On 09/05/2012 02:28 PM, Kyle Mestery (kmestery) wrote: On Sep 5, 2012, at 1:24 PM, Laine Stump wrote: This fixes https://bugzilla.redhat.com/show_bug.cgi?id=852984 If a network or interface is configured to use Open vSwitch, but ovs-vswitchd (the Open vSwitch database service) isn't running, the ovs-vsctl add-port/del-port commands will hang indefinitely rather than returning an error. There is a --nowait option, but that appears to have no effect on add-port and del-port commands, so instead we add a --timeout=5 to the commands - they will retry for up to 5 seconds, then fail if there is no response. This looks good. I agree with your comments here around the 5 second timeout not being ideal, but being workable. Thanks. I pushed it, but feel free to revisit if you come up with a better solution. (I greatly dislike any hardcoded timeout and only use them as a last resort). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: require netcf-0.2.2 when installing on Fedora18+
On 09/04/2012 01:17 PM, Eric Blake wrote: On 09/04/2012 11:10 AM, Laine Stump wrote: A previous patch forced libnl-3 and netcf-0.2.2 (which itself requires libnl-3) when *building* for Fedora 18+ (and RHEL 7+), but the install-time Requires: for netcf has always been implicit due to libvirtd linking with libnetcf.so. However, the since the API of netcf didn't change when it was rebuilt to use libnl-3, the internal library version didn't change either, making it possible (from rpm's point of view) to upgrade libvirt without upgrading netcf (in reality, that leads to a segfault - see https://bugzilla.redhat.com/show_bug.cgi?id=853381). The solution is to put an explicit Requires: line in libvirt's specfile for fedora = 18 and rhel = 7. --- libvirt.spec.in | 3 +++ 1 file changed, 3 insertions(+) ACK. Thanks. Pushed - we should push this same change into the f18 and rawhide builds of libvirt on Fedora (there's even an appropriate BZ, but bugzilla is currently down so I'll have to make a reminder on my todo list to update it later). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix 'make' issues on OpenBSD.
On 09/05/2012 12:02 PM, Eric Blake wrote: Pushing both under the build-breaker rule. 'make check' still fails some gnulib tests on OpenBSD, but that's not quite as severe. Shoot; I was thinking too much about Jasper's patches. I was actually testing on FreeBSD, not OpenBSD, for these particular patches (and for the commits I've already pushed, oh well). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/2] build: avoid test failure when sasl was not compiled in
On systems without cyrus-sasl-devel available (I happened to be in that situation on my FreeBSD testing), this test fails rather miserably: TEST: libvirtdconftest .!!...! 39 FAIL FAIL: libvirtdconftest with verbose output showing things like: 39) Test corruption ... libvir: Config File error : unsupporeted configuration: remoteReadConfigFile: /usr/home/dummy/libvirt/tests/../daemon/libvirtd.conf: auth_tcp: unsupported auth sasl * tests/libvirtdconftest.c (testCorrupt): Avoid failure when sasl is missing. --- Another build-breaker push. tests/libvirtdconftest.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c index b394d54..a6e1f35 100644 --- a/tests/libvirtdconftest.c +++ b/tests/libvirtdconftest.c @@ -120,6 +120,13 @@ testCorrupt(const void *opaque) goto cleanup; } +#if !HAVE_SASL +if (strstr(err-message, unsupported auth sasl)) { +VIR_DEBUG(sasl unsupported, skipping this config); +goto cleanup; +} +#endif + switch (type) { case VIR_CONF_LONG: if (!strstr(err-message, invalid type: got string; expected long)) { -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] broken OpenBSD net/if.h [was: [PATCH] Include some extra headers needed for OpenBSD.]
On 09/04/2012 11:23 AM, Jasper Lievisse Adriaanse wrote: On Tue, Sep 04, 2012 at 11:08:30AM -0600, Eric Blake wrote: [adding gnulib] Ouch. The POSIX definition of net/if.h doesn't include any interface that needs to use struct sockaddr. Which OpenBSD extension function is triggering this warning? According to POSIX, this .c file should compile: #define _POSIX_C_SOURCE 200809L #include net/if.h #include sys/socket.h struct if_nameindex i; It turns out that FreeBSD 8.2 also has a non-self-contained net/if.h; so even though I don't have an OpenBSD box handy today, I was able to test a module for a replacement header that works around the issue. Patch coming up shortly. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: improved handling of execinfo.h, BSD net/if.h
FreeBSD and OpenBSD have a net/if.h that is not self-contained; and mingw lacks the header altogether. But gnulib has just taken care of that for us, so we might as well simplify our code. In the process, I got a syntax-check failure if we don't also take the gnulib execinfo module. * .gnulib: Update to latest, for execinfo and net_if. * bootstrap.conf (gnulib_modules): Add execinfo and net_if modules. * configure.ac: Let gnulib check for headers. Simplify check for 'struct ifreq', while also including enough prereq headers. * src/internal.h (IF_NAMESIZE): Drop, now that gnulib guarantees it. * src/nwfilter/nwfilter_learnipaddr.h: Use correct header for IF_NAMESIZE. * src/util/virnetdev.c (includes): Assume net/if.h exists. * src/util/virnetdevbridge.c (includes): Likewise. * src/util/virnetdevtap.c (includes): Likewise. * src/util/logging.c (includes): Assume execinfo.h exists. (virLogStackTraceToFd): Handle gnulib's fallback implementation. --- Successfully tested on Fedora and FreeBSD; I'm still trying to also test a cross-compile to mingw. Gnulib changes amount to: * .gnulib 271dd74...440a1db (36): net_if: new module readutmp: fix non-portable UT_PID use update from texinfo fts: reduce two or more trailing slashes to just one, usually fts: when there is no risk of overlap, use memcpy, not memmove autoupdate autoupdate manywarnings: update the list of all warnings * lib/stdbool.in.h (_Bool) [__cplusplus]: bool, not _Bool. stdbool: be more compatible with mixed C/C++ compiles revert last change: it was not needed tests: test-vc-list-files-git.sh: skip if git is not available gnulib-tool: Remove no-op option --no-changelog. autoupdate doc: remove fdl-1.2.texi execinfo: port to FreeBSD xstrtol.h: avoid _Noreturn is not at beginning of declaration warning doc: do not use @acronym stdnoreturn: port to newer GCCs pipe-filter: fix comment typo execinfo: new module extern-inline: support old GCC 'inline' maint.mk: avoid redundant file name in message timer-time: fix link order when static linking on glibc timespec: omit unnecessary AC_C_INLINE stat-time: omit unnecessary AC_C_INLINE ignore-value: omit unnecessary AC_C_INLINE sys_select: avoid 'static inline' mktime: avoid 'static inline' autoupdate gnulib-tool: Improve coding style. gnulib-tool: Fix indentation. gnulib-tool: Remove old file names from .cvsignore, .gitignore. test-parse-datetime: avoid glibc leap-second glitch autoupdate gnulib-tool: Fix indentation of generated gnulib-comp.m4 file. .gnulib | 2 +- bootstrap.conf | 2 ++ configure.ac| 23 +++ src/internal.h | 4 src/nwfilter/nwfilter_learnipaddr.h | 2 ++ src/util/logging.c | 22 +- src/util/virnetdev.c| 4 +--- src/util/virnetdevbridge.c | 6 ++ src/util/virnetdevtap.c | 4 +--- 9 files changed, 25 insertions(+), 44 deletions(-) diff --git a/.gnulib b/.gnulib index 271dd74..440a1db 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 271dd74fdf54ec2a03e73a5173b0b5697f6088f1 +Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 diff --git a/bootstrap.conf b/bootstrap.conf index 7fefb69..2847c0b 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -38,6 +38,7 @@ count-one-bits crypto/md5 dirname-lgpl environ +execinfo fclose fcntl fcntl-h @@ -70,6 +71,7 @@ manywarnings mkstemp mkstemps mktempd +net_if netdb nonblocking openpty diff --git a/configure.ac b/configure.ac index cb91e7d..47a72b9 100644 --- a/configure.ac +++ b/configure.ac @@ -186,8 +186,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ - sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h \ - net/if.h execinfo.h]) + sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h]) dnl We need to decide at configure time if libvirt will use real atomic dnl operations (lock free) or emulated ones with a mutex. @@ -245,20 +244,12 @@ AM_CONDITIONAL([WITH_ATOMIC_OPS_PTHREAD],[test $atomic_ops = pthread]) AC_MSG_RESULT([$atomic_ops]) - -AC_MSG_CHECKING([for struct ifreq in net/if.h]) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM( - [[ - #include net/if.h - ]], - [[ - struct ifreq ifr; - ]])],[ - AC_DEFINE([HAVE_STRUCT_IFREQ],[],[Defined if struct ifreq existsin net/if.h]) - AC_MSG_RESULT([yes]) - ],[ - AC_MSG_RESULT([yes]) - ]) +AC_CHECK_TYPE([struct ifreq], + [AC_DEFINE([HAVE_STRUCT_IFREQ],[1], +[Defined if struct ifreq exists in net/if.h])], + [], [[#include sys/socket.h +#include net/if.h + ]]) dnl Our only use of libtasn1.h is in the testsuite, and can be skipped dnl if the header is not
Re: [libvirt] [RFC] [PATCH v2 1/6] add configure option --with-fuse for libvirt
于 2012年09月05日 20:42, Daniel P. Berrange 写道: On Wed, Sep 05, 2012 at 05:41:40PM +0800, Gao feng wrote: Hi Daniel Glauber 于 2012年07月31日 17:27, Daniel P. Berrange 写道: Hi Gao, I'm wondering if you are planning to attend the Linux Plumbers Conference in San Diego at the end of August ? Glauber is going to be giving a talk on precisely the subject of virtualizing /proc in containers which is exactly what your patch is looking at https://blueprints.launchpad.net/lpc/+spec/lpc2012-cont-proc I'll review your patches now, but I think I'd like to wait to hear what Glauber talks about at LPC before we try to merge this support in libvirt, so we have an broadly agreed long term strategy for /proc between all the interested userspace kernel guys. I did not attend the LPC,so can you tell me what's the situation of the /proc virtualization? I think maybe we should just apply this patchset first,and wait for somebody sending patches to implement /proc virtualization. So there were three main approaches discussed 1. FUSE based /proc + a real hidden /.proc. The FUSE /proc provides custom handling of various files like meminfo, otherwise forwards I/O requests through to the hidden /.proc files. This was the original proof of concept. 2. One FUSE filesystem for all containers + a real /proc. Bind mount files from the FUSE filesystem into the container's /proc. This is what Glauber has done. 3. One FUSE filesystem per container + a real /proc. Bind mount files from the FUSE filesystem into the container's /proc. This is what your patch is doing Options 2 3 have a clear a win over option 1 in efficiency terms, since they avoid doubling the I/O required for the majority of files. Glaubar thinks it is perferrable to have a single FUSE filesystem that has one sub-directory for each container. Then bind mount the appropriate sub dir into each container. I kinda like the way you have done things, having a private FUSE filesystem per container, for security reasons. By having the FUSE backend be part of the libvirt_lxc process we have strictly isolated each containers' environment. If we wanted a single shared FUSE for all containers, we'd need to have some single shared daemon to maintain it. This could not be libvirtd itself, since we need the containers their filesystems to continue to work when libvirtd itself is not running. We could introduce a separate libvirt_fused which provided a shared filesystem, but this still has the downside that any flaw in its impl could provide a way for one container to attack another container Agree,if we choose the option 2,we have to organize the sub-directory for each container in fuse,it will make fuse filesystem complicated. So in summary, I think your patches which add a private FUSE per container in libvirt_lxc appear to be the best option at this time. Ok,I will rebase this patchset and send the v3 patchset. Thanks Gao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Add cpu hotplug support to libvirt.
Hi Daniel, On 09/05/2012 05:43 PM, Daniel P. Berrange wrote: Your patch appears to work in some limited scenarios, but more generally it will fail to work, and resulted in undesirable behaviour. Consider for example, if libvirtd is configured thus: cd /sys/fs/cgroup/cpuset mkdir demo cd demo echo 2-3 cpuset.cpus echo 0 cpuset.mems echo $$ tasks /usr/sbin/libvirtd ie, libvirtd is now running on cpus 2-3, in group 'demo'. VMs will be created in /sys/fs/cgroup/cpuset/demo/libvirt/qemu/$VMNAME Your patch attempts to set the cpuset.cpus on 'libvirt/qemu/$VMNAME' but ignores the fact that there could be many higher directories (eg demo here) that need setting. libvirtd, however, should not be responsible for / allowed to change settings in parent cgroups from where it was started. ie in this example, libvirtd should *not* touch the 'demo' cgroup. Yes, I didn't realize this situation. Thanks for remind me. :) So consider systemd starting tasks, giving them custom cgroups. Now systemd also has to listen for netlink events and reset the cpuset masks. Things are even worse if the admin has temporarily offlined all the cpus that are associated with the current cpuset. When this happens the kernel throws libvirtd and all its VMs out of their current cgroups and dumps them up in a parent cgroup (potentially even the root group). This is really awful. Agreed. :) IMHO, execution of those tasks should simply be paused (same way that the 'freezer' cgroup pauses tasks). The admin can then either move the tasks to an alternate cgroup, or change the cpuset mask to allow them to continue running. The kernel's current behaviour of pushing all tasks up into a parent cgroup is just crazy - it is just throwing away the users requested cpu mask forever :-( If I want to solve the start failure problem, what should I do ? I maintain the problems we see with cpuset controller cannot be reasonably solved by libvirtd, or userspace in general. The kernel behaviour is just flawed. If the kernel won't fix it, then we should recommend people not to use the cpuset cgroup at all, and just rely on our sched_setaffinity support instead. I like the sched_setaffinity idea. Let's just temporarily shut off cpuset cgroup in libvirt, shall we ? Since cpuset cgroup was turned on when I was working on the emulator-pin job, I will shut if off and improve all these with sched_setaffinity(). And I will send new patches soon. Thanks. :) Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv6 0/2] Implementation of virConnectListAllDomains() for esx and hyperv
On Fri, Aug 31, 2012 at 05:36:51PM +0200, Peter Krempa wrote: Yet another respin updated and rebased to current head. Both drivers are compile tested but I don't have the infrastructure do a functional test. Peter Krempa (2): hyperv: Add implementation for virConnectListAllDomains() esx: Add implementation for virConnectListAllDomains() src/esx/esx_driver.c | 194 + src/hyperv/hyperv_driver.c | 135 +++ 2 files changed, 329 insertions(+) I was hoping to see feedback on actual users, but since we are at v6 and nobody replied, let's push them and see ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list