January 2008 - Devel - libvirt List Archives

[Libvir] PATCH: Add missing APIs to python binding
by Daniel P. Berrange 21 Jan '08

21 Jan '08

The python binding is missing all the APIs relating to CPU pinning and schedular parameters because they are too hard for the generator to manage. This patch implements them manually. Now a few words about the impls since they are non-trivial * virDomainGetSchedulerType The C binding has a return value, and an out parameter. I translated this into a tuple in python (schedular-type, n-parms) eg print dom.schedulerType() ('credit', 2) Arguably we could drop the nparams field - its less use in python since you don't have to pre-allocate a virSchedParam list. * virDomainGetSchedulerParameters This has an out parameter which is a list of virSchedParam structs, and a param specifying the length of the passed in virSchedParam list. The latter is irrelevant; The former I translated into a hash table return value eg print dom.schedulerParameters() {'cap': 0, 'weight': 100} * virDomainSetSchedulerParameters This has two in parameters, a list of virSchedParma structs, and a param specifying length of the list. I just convert this into a hash. I also make all the fields optional - it'll copy existing values for any params which are omimtted. eg dom.setSchedulerParameters({'weight': 100}) * virDomainGetVcpus This has two out parameters, a list of virVcpuInfo structs, and a bitmap both pre-allocated, and their size passed in. This needs to be turned into a 2 element tuple return value. First element returning vcpu info, and the second element returning mapping. The vcpu info will be returned as a list of lists, and the cpumap will be another list of lists - a True indicating affinity. eg (cpuinfo, cpumap) = dom.vcpus() print cpuinfo print cpumap [(0, 2, 7753916026L, 1), (1, 2, 5698028782L, 0), (2, 2, 4916522979L, 0), (3, 2, 3773416711L, 1)] [(False, True), (True, True), (True, False), (True, True)] * virDomainPinVcpu This takes a vcpu number and a bitmap of affinity. I turn the bitmap into a list of True/False values eg dom.pinVcpu(1, (True, True)) The generator fails on the C code part of the bindings completely, so that is hand-written. It also fails on the python part of the bindings, but the way the generator is structured doesn't enable us to hand-write the python part for methods within objects :-( So I have basically just editted the generator to blacklist all the out-parameters, and blacklist the in parameters which specify list lengths. Here is the code I used to test the functions: #!/usr/bin/python import libvirt con = libvirt.open("xen:///") dom = con.lookupByName("rhel5pv") dominfo = dom.info() nodeinfo = con.getInfo() pcpus = nodeinfo[4] * nodeinfo[5] * nodeinfo[6] * nodeinfo[7] (cpuinfo, cpumap) = dom.vcpus() print cpuinfo print cpumap for i in range(dominfo[3]): print "vcpu: %d" % i if cpuinfo[i][1] == 0: print "state: offline" elif cpuinfo[i][1] == 2: print "state: blocked" else: print "state: running" print "cputime: %d" % cpuinfo[i][2] print "pcpu: %d" % cpuinfo[i][3] affinity = "" for p in range(pcpus): if cpumap[i][p] == True: affinity = affinity + "y" else: affinity = affinity + "-" print "affinity: %s" % affinity print dom.pinVcpu(1, (True, True)) print dom.schedulerType() print dom.schedulerParameters() dom.setSchedulerParameters({"weight": 100}) And here is the diffstat: generator.py | 26 ++++ libvir.c | 318 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 344 insertions(+) Dan. diff -r 8468c0d428c9 python/generator.py --- a/python/generator.py Sun Jan 20 12:06:47 2008 -0500 +++ b/python/generator.py Sun Jan 20 12:07:59 2008 -0500 @@ -229,6 +229,7 @@ py_types = { 'double': ('d', None, "double", "double"), 'unsigned int': ('i', None, "int", "int"), 'unsigned long': ('l', None, "long", "long"), + 'unsigned long long': ('l', None, "longlong", "long long"), 'unsigned char *': ('z', None, "charPtr", "char *"), 'char *': ('z', None, "charPtr", "char *"), 'const char *': ('z', None, "charPtrConst", "const char *"), @@ -279,6 +280,11 @@ skip_impl = ( 'virDomainBlockStats', 'virDomainInterfaceStats', 'virNodeGetCellsFreeMemory', + 'virDomainGetSchedulerType', + 'virDomainGetSchedulerParameters', + 'virDomainSetSchedulerParameters', + 'virDomainGetVcpus', + 'virDomainPinVcpu', ) def skip_function(name): @@ -918,6 +924,16 @@ def buildWrappers(): txt.write(" %s()\n" % func); n = 0 for arg in args: + if name == "virDomainPinVcpu" and arg[0] == "maplen": + continue + if name == "virDomainGetSchedulerType" and arg[0] == "nparams": + continue + if name == "virDomainSetSchedulerParameters" and arg[0] == "nparams": + continue + if name == "virDomainGetSchedulerParameters" and arg[0] != "domain": + continue + if name == "virDomainGetVcpus" and arg[0] != "domain": + continue if n != index: classes.write(", %s" % arg[0]) n = n + 1 @@ -939,6 +955,16 @@ def buildWrappers(): classes.write("libvirtmod.%s(" % name) n = 0 for arg in args: + if name == "virDomainPinVcpu" and arg[0] == "maplen": + continue + if name == "virDomainGetSchedulerType" and arg[0] == "nparams": + continue + if name == "virDomainSetSchedulerParameters" and arg[0] == "nparams": + continue + if name == "virDomainGetSchedulerParameters" and arg[0] != "domain": + continue + if name == "virDomainGetVcpus" and arg[0] != "domain": + continue if n != 0: classes.write(", "); if n != index: diff -r 8468c0d428c9 python/libvir.c --- a/python/libvir.c Sun Jan 20 12:06:47 2008 -0500 +++ b/python/libvir.c Sun Jan 20 12:07:59 2008 -0500 @@ -102,6 +102,319 @@ libvirt_virDomainInterfaceStats(PyObject PyTuple_SetItem(info, 7, PyLong_FromLongLong(stats.tx_drop)); return(info); } + + +static PyObject * +libvirt_virDomainGetSchedulerType(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + char *c_retval; + int nparams; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetScedulerType", + &pyobj_domain)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + c_retval = virDomainGetSchedulerType(domain, &nparams); + if (c_retval == NULL) + return VIR_PY_NONE; + + /* convert to a Python tupple of long objects */ + if ((info = PyTuple_New(2)) == NULL) { + free(c_retval); + return VIR_PY_NONE; + } + + PyTuple_SetItem(info, 0, libvirt_constcharPtrWrap(c_retval)); + PyTuple_SetItem(info, 1, PyInt_FromLong((long)nparams)); + free(c_retval); + return(info); +} + +static PyObject * +libvirt_virDomainGetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + char *c_retval; + int nparams, i; + virSchedParameterPtr params; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetScedulerParameters", + &pyobj_domain)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + c_retval = virDomainGetSchedulerType(domain, &nparams); + if (c_retval == NULL) + return VIR_PY_NONE; + free(c_retval); + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return VIR_PY_NONE; + + if (virDomainGetSchedulerParameters(domain, params, &nparams) < 0) { + free(params); + return VIR_PY_NONE; + } + + /* convert to a Python tupple of long objects */ + if ((info = PyDict_New()) == NULL) { + free(params); + return VIR_PY_NONE; + } + for (i = 0 ; i < nparams ; i++) { + PyObject *key, *val; + + switch (params[i].type) { + case VIR_DOMAIN_SCHED_FIELD_INT: + val = PyInt_FromLong((long)params[i].value.i); + break; + + case VIR_DOMAIN_SCHED_FIELD_UINT: + val = PyInt_FromLong((long)params[i].value.ui); + break; + + case VIR_DOMAIN_SCHED_FIELD_LLONG: + val = PyLong_FromLongLong((long long)params[i].value.l); + break; + + case VIR_DOMAIN_SCHED_FIELD_ULLONG: + val = PyLong_FromLongLong((long long)params[i].value.ul); + break; + + case VIR_DOMAIN_SCHED_FIELD_DOUBLE: + val = PyFloat_FromDouble((double)params[i].value.d); + break; + + case VIR_DOMAIN_SCHED_FIELD_BOOLEAN: + val = PyBool_FromLong((long)params[i].value.b); + break; + + default: + free(params); + Py_DECREF(info); + return VIR_PY_NONE; + } + + key = libvirt_constcharPtrWrap(params[i].field); + PyDict_SetItem(info, key, val); + } + free(params); + return(info); +} + +static PyObject * +libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *info; + char *c_retval; + int nparams, i; + virSchedParameterPtr params; + + if (!PyArg_ParseTuple(args, (char *)"OO:virDomainSetScedulerParameters", + &pyobj_domain, &info)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + c_retval = virDomainGetSchedulerType(domain, &nparams); + if (c_retval == NULL) + return VIR_PY_NONE; + free(c_retval); + + if ((params = malloc(sizeof(*params)*nparams)) == NULL) + return VIR_PY_NONE; + + if (virDomainGetSchedulerParameters(domain, params, &nparams) < 0) { + free(params); + return VIR_PY_NONE; + } + + /* convert to a Python tupple of long objects */ + for (i = 0 ; i < nparams ; i++) { + PyObject *key, *val; + key = libvirt_constcharPtrWrap(params[i].field); + val = PyDict_GetItem(info, key); + Py_DECREF(key); + + if (val == NULL) + continue; + + switch (params[i].type) { + case VIR_DOMAIN_SCHED_FIELD_INT: + params[i].value.i = (int)PyInt_AS_LONG(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_UINT: + params[i].value.ui = (unsigned int)PyInt_AS_LONG(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_LLONG: + params[i].value.l = (long long)PyLong_AsLongLong(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_ULLONG: + params[i].value.ul = (unsigned long long)PyLong_AsLongLong(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_DOUBLE: + params[i].value.d = (double)PyFloat_AsDouble(val); + break; + + case VIR_DOMAIN_SCHED_FIELD_BOOLEAN: + { + /* Hack - Python's definition of Py_True breaks strict + * aliasing rules, so can't directly compare :-( + */ + PyObject *hacktrue = PyBool_FromLong(1); + params[i].value.b = hacktrue == val ? 1 : 0; + Py_DECREF(hacktrue); + } + break; + + default: + free(params); + return VIR_PY_NONE; + } + } + + if (virDomainSetSchedulerParameters(domain, params, nparams) < 0) { + free(params); + return VIR_PY_NONE; + } + + free(params); + return VIR_PY_NONE; +} + +static PyObject * +libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *pyretval = NULL, *pycpuinfo = NULL, *pycpumap = NULL; + virNodeInfo nodeinfo; + virDomainInfo dominfo; + virVcpuInfoPtr cpuinfo = NULL; + unsigned char *cpumap = NULL; + int cpumaplen, i; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetVcpus", + &pyobj_domain)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if (virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo) != 0) + return VIR_PY_NONE; + + if (virDomainGetInfo(domain, &dominfo) != 0) + return VIR_PY_NONE; + + if ((cpuinfo = malloc(sizeof(*cpuinfo)*dominfo.nrVirtCpu)) == NULL) + return VIR_PY_NONE; + + cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + if ((cpumap = malloc(dominfo.nrVirtCpu * cpumaplen)) == NULL) + goto cleanup; + + if (virDomainGetVcpus(domain, + cpuinfo, dominfo.nrVirtCpu, + cpumap, cpumaplen) < 0) + goto cleanup; + + /* convert to a Python tupple of long objects */ + if ((pyretval = PyTuple_New(2)) == NULL) + goto cleanup; + if ((pycpuinfo = PyList_New(dominfo.nrVirtCpu)) == NULL) + goto cleanup; + if ((pycpumap = PyList_New(dominfo.nrVirtCpu)) == NULL) + goto cleanup; + + for (i = 0 ; i < dominfo.nrVirtCpu ; i++) { + PyObject *info = PyTuple_New(4); + if (info == NULL) + goto cleanup; + PyTuple_SetItem(info, 0, PyInt_FromLong((long)cpuinfo[i].number)); + PyTuple_SetItem(info, 1, PyInt_FromLong((long)cpuinfo[i].state)); + PyTuple_SetItem(info, 2, PyLong_FromLongLong((long long)cpuinfo[i].cpuTime)); + PyTuple_SetItem(info, 3, PyInt_FromLong((long)cpuinfo[i].cpu)); + PyList_SetItem(pycpuinfo, i, info); + } + for (i = 0 ; i < dominfo.nrVirtCpu ; i++) { + PyObject *info = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo)); + int j; + if (info == NULL) + goto cleanup; + for (j = 0 ; j < VIR_NODEINFO_MAXCPUS(nodeinfo) ; j++) { + PyTuple_SetItem(info, j, PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, i, j))); + } + PyList_SetItem(pycpumap, i, info); + } + PyTuple_SetItem(pyretval, 0, pycpuinfo); + PyTuple_SetItem(pyretval, 1, pycpumap); + + free(cpuinfo); + free(cpumap); + + return(pyretval); + + cleanup: + free(cpuinfo); + free(cpumap); + /* NB, Py_DECREF is a badly defined macro, so we require + * braces here to avoid 'ambiguous else' warnings from + * the compiler. + * NB. this comment is true at of time of writing wrt to + * at least python2.5. + */ + if (pyretval) { Py_DECREF(pyretval); } + if (pycpuinfo) { Py_DECREF(pycpuinfo); } + if (pycpumap) { Py_DECREF(pycpumap); } + return VIR_PY_NONE; +} + + +static PyObject * +libvirt_virDomainPinVcpu(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain, *pycpumap, *truth; + virNodeInfo nodeinfo; + unsigned char *cpumap; + int cpumaplen, i, vcpu; + + if (!PyArg_ParseTuple(args, (char *)"OiO:virDomainPinVcpu", + &pyobj_domain, &vcpu, &pycpumap)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if (virNodeGetInfo(virDomainGetConnect(domain), &nodeinfo) != 0) + return VIR_PY_NONE; + + cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); + if ((cpumap = malloc(cpumaplen)) == NULL) + return VIR_PY_NONE; + memset(cpumap, 0, cpumaplen); + + truth = PyBool_FromLong(1); + for (i = 0 ; i < VIR_NODEINFO_MAXCPUS(nodeinfo) ; i++) { + PyObject *flag = PyTuple_GetItem(pycpumap, i); + if (flag == truth) + VIR_USE_CPU(cpumap, i); + else + VIR_UNUSE_CPU(cpumap, i); + } + + virDomainPinVcpu(domain, vcpu, cpumap, cpumaplen); + Py_DECREF(truth); + free(cpumap); + + return VIR_PY_NONE; +} + + /************************************************************************ * * * Global error handler at the Python level * @@ -876,6 +1189,11 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainBlockStats", libvirt_virDomainBlockStats, METH_VARARGS, NULL}, {(char *) "virDomainInterfaceStats", libvirt_virDomainInterfaceStats, METH_VARARGS, NULL}, {(char *) "virNodeGetCellsFreeMemory", libvirt_virNodeGetCellsFreeMemory, METH_VARARGS, NULL}, + {(char *) "virDomainGetSchedulerType", libvirt_virDomainGetSchedulerType, METH_VARARGS, NULL}, + {(char *) "virDomainGetSchedulerParameters", libvirt_virDomainGetSchedulerParameters, METH_VARARGS, NULL}, + {(char *) "virDomainSetSchedulerParameters", libvirt_virDomainSetSchedulerParameters, METH_VARARGS, NULL}, + {(char *) "virDomainGetVcpus", libvirt_virDomainGetVcpus, METH_VARARGS, NULL}, + {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} }; -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

3 7

[Libvir] PATCH: Remove no-op network API impl from OpenVZ driver
by Daniel P. Berrange 21 Jan '08

21 Jan '08

The openvz_driver.c file registers an implementation of the networking driver APIs, which is a no-op. Even though the networking APIs are implemented in the QEMU driver, this one impl is *shared* and used across all virtualization driver backends. Thus no other driver such as Xen/OpenVZ/etc should ever implement the network APIs. The only exceptions are the remote driver, and the test driver. So this patch removes the no-op impl from openvz diff -r 079dc4e2fd54 src/openvz_driver.c --- a/src/openvz_driver.c Mon Jan 14 18:09:49 2008 -0500 +++ b/src/openvz_driver.c Mon Jan 14 19:04:10 2008 -0500 @@ -86,14 +86,6 @@ static int openvzShutdown(void); static int openvzShutdown(void); static int openvzReload(void); static int openvzActive(void); -static int openvzCloseNetwork(virConnectPtr conn); -static virDrvOpenStatus openvzOpenNetwork(virConnectPtr conn, - const char *name ATTRIBUTE_UNUSED, - int *credtype ATTRIBUTE_UNUSED, - int ncredtype ATTRIBUTE_UNUSED, - virConnectAuthCallbackPtr cb ATTRIBUTE_UNUSED, - void *cbdata ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED); static virDomainPtr openvzDomainDefineXML(virConnectPtr conn, const char *xml); static virDomainPtr openvzDomainCreateLinux(virConnectPtr conn, const char *xml, @@ -696,20 +688,6 @@ static int openvzActive(void) { static int openvzActive(void) { return 1; -} - -static int openvzCloseNetwork(virConnectPtr conn ATTRIBUTE_UNUSED) { - return 0; -} - -static virDrvOpenStatus openvzOpenNetwork(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *name ATTRIBUTE_UNUSED, - int *credtype ATTRIBUTE_UNUSED, - int ncredtype ATTRIBUTE_UNUSED, - virConnectAuthCallbackPtr cb ATTRIBUTE_UNUSED, - void *cbdata ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED) { - return VIR_DRV_OPEN_SUCCESS; } static virDriver openvzDriver = { @@ -771,27 +749,6 @@ static virDriver openvzDriver = { NULL, /* nodeGetFreeMemory */ }; -static virNetworkDriver openvzNetworkDriver = { - NULL, /* name */ - openvzOpenNetwork, /* open */ - openvzCloseNetwork, /* close */ - NULL, /* numOfNetworks */ - NULL, /* listNetworks */ - NULL, /* numOfDefinedNetworks */ - NULL, /* listDefinedNetworks */ - NULL, /* networkLookupByUUID */ - NULL, /* networkLookupByName */ - NULL, /* networkCreateXML */ - NULL, /* networkDefineXML */ - NULL, /* networkUndefine */ - NULL, /* networkCreate */ - NULL, /* networkDestroy */ - NULL, /* networkDumpXML */ - NULL, /* networkGetBridgeName */ - NULL, /* networkGetAutostart */ - NULL, /* networkSetAutostart */ -}; - static virStateDriver openvzStateDriver = { openvzStartup, openvzShutdown, @@ -801,7 +758,6 @@ static virStateDriver openvzStateDriver int openvzRegister(void) { virRegisterDriver(&openvzDriver); - virRegisterNetworkDriver(&openvzNetworkDriver); virRegisterStateDriver(&openvzStateDriver); return 0; } Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

3 3

[Libvir] PATCH: Make virsh use virFileReadAll
by Daniel P. Berrange 21 Jan '08

21 Jan '08

This patch makes virsh use the virFileReadAll API from util.c, instead of duplicating it. This means we need to export this symbol from the library. diff -r 1eb02a424697 src/libvirt_sym.version --- a/src/libvirt_sym.version Sat Jan 19 13:45:07 2008 -0500 +++ b/src/libvirt_sym.version Sat Jan 19 14:28:41 2008 -0500 @@ -131,5 +131,7 @@ __virDomainMigratePerform; __virDomainMigrateFinish; + __virFileReadAll; + local: *; }; diff -r 1eb02a424697 src/util.c --- a/src/util.c Sat Jan 19 13:45:07 2008 -0500 +++ b/src/util.c Sat Jan 19 14:28:41 2008 -0500 @@ -316,9 +316,9 @@ ssize_t safewrite(int fd, const void *bu } -int virFileReadAll(const char *path, - int maxlen, - char **buf) +int __virFileReadAll(const char *path, + int maxlen, + char **buf) { FILE *fh; struct stat st; diff -r 1eb02a424697 src/util.h --- a/src/util.h Sat Jan 19 13:45:07 2008 -0500 +++ b/src/util.h Sat Jan 19 14:28:41 2008 -0500 @@ -33,9 +33,10 @@ int saferead(int fd, void *buf, size_t c int saferead(int fd, void *buf, size_t count); ssize_t safewrite(int fd, const void *buf, size_t count); -int virFileReadAll(const char *path, - int maxlen, - char **buf); +int __virFileReadAll(const char *path, + int maxlen, + char **buf); +#define virFileReadAll(p,m,b) __virFileReadAll((p),(m),(b)) int virFileMatchesNameSuffix(const char *file, const char *name, diff -r 1eb02a424697 src/virsh.c --- a/src/virsh.c Sat Jan 19 13:45:07 2008 -0500 +++ b/src/virsh.c Sat Jan 19 14:28:41 2008 -0500 @@ -48,6 +48,7 @@ #include "internal.h" #include "console.h" +#include "util.h" static char *progname; @@ -55,6 +56,8 @@ static char *progname; #define TRUE 1 #define FALSE 0 #endif + +#define VIRSH_MAX_XML_FILE 10*1024*1024 #define VSH_PROMPT_RW "virsh # " #define VSH_PROMPT_RO "virsh > " @@ -858,66 +861,6 @@ static vshCmdOptDef opts_create[] = { {NULL, 0, 0, NULL} }; -/* Read in a whole file and return it as a string. - * If it fails, it logs an error and returns NULL. - * String must be freed by caller. - */ -static char * -readFile (vshControl *ctl, const char *filename) -{ - char *retval; - int len = 0, fd; - - if ((fd = open(filename, O_RDONLY)) == -1) { - vshError (ctl, FALSE, _("Failed to open '%s': %s"), - filename, strerror (errno)); - return NULL; - } - - if (!(retval = malloc(len + 1))) - goto out_of_memory; - - while (1) { - char buffer[1024]; - char *new; - int ret; - - if ((ret = read(fd, buffer, sizeof(buffer))) == 0) - break; - - if (ret == -1) { - if (errno == EINTR) - continue; - - vshError (ctl, FALSE, _("Failed to open '%s': read: %s"), - filename, strerror (errno)); - goto error; - } - - if (!(new = realloc(retval, len + ret + 1))) - goto out_of_memory; - - retval = new; - - memcpy(retval + len, buffer, ret); - len += ret; - } - - retval[len] = '\0'; - return retval; - - out_of_memory: - vshError (ctl, FALSE, _("Error allocating memory: %s"), - strerror(errno)); - - error: - if (retval) - free(retval); - close(fd); - - return NULL; -} - static int cmdCreate(vshControl * ctl, vshCmd * cmd) { @@ -934,8 +877,8 @@ cmdCreate(vshControl * ctl, vshCmd * cmd if (!found) return FALSE; - buffer = readFile (ctl, from); - if (buffer == NULL) return FALSE; + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; dom = virDomainCreateLinux(ctl->conn, buffer, 0); free (buffer); @@ -982,8 +925,8 @@ cmdDefine(vshControl * ctl, vshCmd * cmd if (!found) return FALSE; - buffer = readFile (ctl, from); - if (buffer == NULL) return FALSE; + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; dom = virDomainDefineXML(ctl->conn, buffer); free (buffer); @@ -2372,8 +2315,8 @@ cmdNetworkCreate(vshControl * ctl, vshCm if (!found) return FALSE; - buffer = readFile (ctl, from); - if (buffer == NULL) return FALSE; + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; network = virNetworkCreateXML(ctl->conn, buffer); free (buffer); @@ -2420,8 +2363,8 @@ cmdNetworkDefine(vshControl * ctl, vshCm if (!found) return FALSE; - buffer = readFile (ctl, from); - if (buffer == NULL) return FALSE; + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; network = virNetworkDefineXML(ctl->conn, buffer); free (buffer); @@ -3107,8 +3050,8 @@ cmdAttachDevice(vshControl * ctl, vshCmd return FALSE; } - buffer = readFile (ctl, from); - if (buffer == NULL) return FALSE; + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; ret = virDomainAttachDevice(dom, buffer); free (buffer); @@ -3161,8 +3104,8 @@ cmdDetachDevice(vshControl * ctl, vshCmd return FALSE; } - buffer = readFile (ctl, from); - if (buffer == NULL) return FALSE; + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; ret = virDomainDetachDevice(dom, buffer); free (buffer); Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

3 3

[Libvir] PATCH: Fix and cleanup ref counting/ domain/network object release
by Daniel P. Berrange 21 Jan '08

21 Jan '08

The referencing counting code for Connect/Domain/Network objects has many repeated codepaths, not all of which are correct. eg, the virFreeDomain method forgets to release networks when garbage collecting a virConnectPtr, and the virFreeNetwork method forgets to release domains. I've also found it very confusing having both virFreeDomain and virDomainFree. So I've changed the impl of the cleanup functions in hash.c to use a slightly different structure. There are now 6 functions: - virUnrefConnect - virReleaseConnect - virUnrefDomain - virReleaseDomain - virUnrefNetwork The 'virUnref*' APIs are intended for use by the drivers to decrement the ref count on objects they no loner need. In the virUnref* method, if the ref count hits zero, then it calls the corresponding virRelease* method to actually free the object's memory. The virUnref* methods are responsible for acquiring the connection mutex. The virRelease* method mandate that5D the mutex is already held, and will releae it as the very last step when deleting the object. The virReleaseDomain/virReleaeNetwork methods will also derecement the ref count of the connection, and call virReleaseConnection if appropriate. The patch for all this looks horrific but its not as bad as it looks, so I'd recommend reviewing the final code in hash.c, rather than the patch. I have also switched from xmlMutex to the POSIX standard pthread_mutex_t object. In theory the former may give more portability, but the patches which follow are also going to require actual pthread_t objects, for which libxml has no wrappers. Thus it is pointless using the xmlMutex abstraction. The added plus, is that pthread_mutex_t objects do not require any memory allocation, merely initialization of their pre-allocated contents so it simplifies a little code. Finally, the code in hash.c for dealing with ref counting seriously abused the 'goto' statement with multiple jumps overlapping each other in a single method. This made it very hard to follow. So I removed the use of goto in most places so its only used in error cleanup paths, and not for regular flow control. Oh, and in cleaning up internal.h I found an odd declaration of some functions from xs_internal.h which could have been made static. So I made them static. In this version I removed the call to 'virUnrefDomain' from the libvirt.c file's virDomainDestroy call, and likewise in virNetworkDestroy. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

3 5

[Libvir] PATCH: const-correct sexpr
by Jim Meyering 21 Jan '08

21 Jan '08

I was looking at xend_internal.c and wondered why sexpr_int's sexpr pointer wasn't const... surely, it *can't* modify that, I thought. So I made it const, and pulled the thread, which ended up making most of the sexpr* parameters in sexpr.[ch] const. I added the few inevitable casts, but imho, that cost is far outweighed by having accurate prototypes for all of these functions. Signed-off-by: Jim Meyering <meyering(a)redhat.com> --- src/sexpr.c | 26 +++++++++++++------------- src/sexpr.h | 12 ++++++------ src/xend_internal.c | 18 ++++++++++-------- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/sexpr.c b/src/sexpr.c index f9e7226..b273117 100644 --- a/src/sexpr.c +++ b/src/sexpr.c @@ -150,15 +150,15 @@ sexpr_string(const char *str, ssize_t len) * Returns the resulting S-Expression pointer or NULL in case of error. */ struct sexpr * -sexpr_cons(struct sexpr *car, struct sexpr *cdr) +sexpr_cons(const struct sexpr *car, const struct sexpr *cdr) { struct sexpr *ret = sexpr_new(); if (ret == NULL) return ret; ret->kind = SEXPR_CONS; - ret->u.s.car = car; - ret->u.s.cdr = cdr; + ret->u.s.car = (struct sexpr *) car; + ret->u.s.cdr = (struct sexpr *) cdr; return ret; } @@ -171,14 +171,14 @@ sexpr_cons(struct sexpr *car, struct sexpr *cdr) * Internal operation appending a value at the end of an existing list */ static void -append(struct sexpr *lst, struct sexpr *value) +append(struct sexpr *lst, const struct sexpr *value) { while (lst->kind != SEXPR_NIL) { lst = lst->u.s.cdr; } lst->kind = SEXPR_CONS; - lst->u.s.car = value; + lst->u.s.car = (struct sexpr *) value; lst->u.s.cdr = sexpr_nil(); } @@ -191,7 +191,7 @@ append(struct sexpr *lst, struct sexpr *value) * Returns lst or NULL in case of error */ struct sexpr * -sexpr_append(struct sexpr *lst, struct sexpr *value) +sexpr_append(struct sexpr *lst, const struct sexpr *value) { if (lst == NULL) return (NULL); @@ -215,7 +215,7 @@ sexpr_append(struct sexpr *lst, struct sexpr *value) * 0 in case of error. */ size_t -sexpr2string(struct sexpr * sexpr, char *buffer, size_t n_buffer) +sexpr2string(const struct sexpr * sexpr, char *buffer, size_t n_buffer) { size_t ret = 0, tmp; @@ -415,7 +415,7 @@ string2sexpr(const char *buffer) * Returns the pointer to the sub expression or NULL if not found. */ static struct sexpr * -sexpr_lookup_key(struct sexpr *sexpr, const char *node) +sexpr_lookup_key(const struct sexpr *sexpr, const char *node) { char buffer[4096], *ptr, *token; @@ -436,7 +436,7 @@ sexpr_lookup_key(struct sexpr *sexpr, const char *node) } for (token = strsep(&ptr, "/"); token; token = strsep(&ptr, "/")) { - struct sexpr *i; + const struct sexpr *i; if (token == NULL) continue; @@ -464,7 +464,7 @@ sexpr_lookup_key(struct sexpr *sexpr, const char *node) return NULL; } - return sexpr; + return (struct sexpr *) sexpr; } /** @@ -478,7 +478,7 @@ sexpr_lookup_key(struct sexpr *sexpr, const char *node) * Returns the pointer to the sub expression or NULL if not found. */ struct sexpr * -sexpr_lookup(struct sexpr *sexpr, const char *node) +sexpr_lookup(const struct sexpr *sexpr, const char *node) { struct sexpr *s = sexpr_lookup_key(sexpr, node); @@ -528,7 +528,7 @@ sexpr_has(struct sexpr *sexpr, const char *node) * Returns the value of the node or NULL if not found. */ const char * -sexpr_node(struct sexpr *sexpr, const char *node) +sexpr_node(const struct sexpr *sexpr, const char *node) { struct sexpr *n = sexpr_lookup(sexpr, node); @@ -547,7 +547,7 @@ sexpr_node(struct sexpr *sexpr, const char *node) * Returns the value of the node or NULL if not found. */ const char * -sexpr_fmt_node(struct sexpr *sexpr, const char *fmt, ...) +sexpr_fmt_node(const struct sexpr *sexpr, const char *fmt, ...) { va_list ap; char node[4096]; diff --git a/src/sexpr.h b/src/sexpr.h index eb82479..0dd882d 100644 --- a/src/sexpr.h +++ b/src/sexpr.h @@ -35,20 +35,20 @@ struct sexpr { }; /* conversion to/from strings */ -size_t sexpr2string(struct sexpr *sexpr, char *buffer, size_t n_buffer); +size_t sexpr2string(const struct sexpr *sexpr, char *buffer, size_t n_buffer); struct sexpr *string2sexpr(const char *buffer); /* constructors and destructors */ struct sexpr *sexpr_nil(void); struct sexpr *sexpr_string(const char *str, ssize_t len); -struct sexpr *sexpr_cons(struct sexpr *car, struct sexpr *cdr); -struct sexpr *sexpr_append(struct sexpr *lst, struct sexpr *item); +struct sexpr *sexpr_cons(const struct sexpr *car, const struct sexpr *cdr); +struct sexpr *sexpr_append(struct sexpr *lst, const struct sexpr *item); void sexpr_free(struct sexpr *sexpr); /* lookup in S-Expressions */ -const char *sexpr_node(struct sexpr *sexpr, const char *node); -const char *sexpr_fmt_node(struct sexpr *sexpr, const char *fmt, ...) +const char *sexpr_node(const struct sexpr *sexpr, const char *node); +const char *sexpr_fmt_node(const struct sexpr *sexpr, const char *fmt, ...) ATTRIBUTE_FORMAT(printf,2,3); -struct sexpr *sexpr_lookup(struct sexpr *sexpr, const char *node); +struct sexpr *sexpr_lookup(const struct sexpr *sexpr, const char *node); int sexpr_has(struct sexpr *sexpr, const char *node); #endif diff --git a/src/xend_internal.c b/src/xend_internal.c index bff625b..f1389c8 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -730,7 +730,7 @@ sexpr_get(virConnectPtr xend, const char *fmt, ...) * Returns the value found or 0 if not found (but may not be an error) */ static int -sexpr_int(struct sexpr *sexpr, const char *name) +sexpr_int(const struct sexpr *sexpr, const char *name) { const char *value = sexpr_node(sexpr, name); @@ -751,7 +751,7 @@ sexpr_int(struct sexpr *sexpr, const char *name) * Returns the value found or 0 if not found (but may not be an error) */ static double -sexpr_float(struct sexpr *sexpr, const char *name) +sexpr_float(const struct sexpr *sexpr, const char *name) { const char *value = sexpr_node(sexpr, name); @@ -772,7 +772,7 @@ sexpr_float(struct sexpr *sexpr, const char *name) * Returns the value found or 0 if not found (but may not be an error) */ static uint64_t -sexpr_u64(struct sexpr *sexpr, const char *name) +sexpr_u64(const struct sexpr *sexpr, const char *name) { const char *value = sexpr_node(sexpr, name); @@ -794,7 +794,7 @@ sexpr_u64(struct sexpr *sexpr, const char *name) * Returns a -1 on error, 0 on success */ static int -sexpr_uuid(unsigned char *ptr, struct sexpr *node, const char *path) +sexpr_uuid(unsigned char *ptr, const struct sexpr *node, const char *path) { const char *r = sexpr_node(node, path); if (!r) @@ -1840,7 +1840,8 @@ xend_parse_domain_sexp(virConnectPtr conn, char *sexpr, int xendConfigVersion) { * Returns 0 in case of success, -1 in case of error */ static int -sexpr_to_xend_domain_info(virDomainPtr domain, struct sexpr *root, virDomainInfoPtr info) +sexpr_to_xend_domain_info(virDomainPtr domain, const struct sexpr *root, + virDomainInfoPtr info) { const char *flags; @@ -1889,7 +1890,7 @@ sexpr_to_xend_domain_info(virDomainPtr domain, struct sexpr *root, virDomainInfo * Returns 0 in case of success, -1 in case of error */ static int -sexpr_to_xend_node_info(struct sexpr *root, virNodeInfoPtr info) +sexpr_to_xend_node_info(const struct sexpr *root, virNodeInfoPtr info) { const char *machine; @@ -1939,7 +1940,8 @@ sexpr_to_xend_node_info(struct sexpr *root, virNodeInfoPtr info) * Returns 0 in case of success, -1 in case of error */ static int -sexpr_to_xend_topology_xml(virConnectPtr conn, struct sexpr *root, virBufferPtr xml) +sexpr_to_xend_topology_xml(virConnectPtr conn, const struct sexpr *root, + virBufferPtr xml) { const char *nodeToCpu; int numCells = 0; @@ -1992,7 +1994,7 @@ error: * Returns the domain pointer or NULL in case of error. */ static virDomainPtr -sexpr_to_domain(virConnectPtr conn, struct sexpr *root) +sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) { virDomainPtr ret = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; -- 1.5.4.rc3.14.g44397

2 4

[Libvir] [PATCH] Don't access line[-1] for a zero-length "line" from fgets.
by Jim Meyering 21 Jan '08

21 Jan '08

In reviewing the StorageAPI patches, I saw that some bogus input could cause a segfault. There's similar code in libvirt.c, so this fixes both. Note that virsh.c does this, too, but it already has the required guard. The src/libvirt.c patch applies to the trunk, but isn't a big deal, since it's in the sample AuthCallback function. The storage_backend.c part applies to the work-in-progress mercurial queue, but does matter, because we have no guarantee what the exec'd command will output. Don't access line[-1] for a zero-length "line" from fgets. A NUL byte at beginning of input, or just after a newline would provoke an invalid buf[-1] access (possible segfault). * src/libvirt.c (virConnectAuthCallbackDefault): * src/storage_backend.c (virStorageBackendRunProgRegex): Signed-off-by: Jim Meyering <meyering(a)redhat.com> --- src/libvirt.c | 6 ++++-- src/storage_backend.c | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 2972382..331c937 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -74,6 +74,7 @@ static int virConnectAuthCallbackDefault(virConnectCredentialPtr cred, for (i = 0 ; i < ncred ; i++) { char buf[1024]; char *bufptr = buf; + size_t len; if (printf("%s:", cred[i].prompt) < 0) return -1; @@ -92,8 +93,9 @@ static int virConnectAuthCallbackDefault(virConnectCredentialPtr cred, } return -1; } - if (buf[strlen(buf)-1] == '\n') - buf[strlen(buf)-1] = '\0'; + len = strlen(buf); + if (len != 0 && buf[len-1] == '\n') + buf[len-1] = '\0'; break; case VIR_CRED_PASSPHRASE: diff --git a/src/storage_backend.c b/src/storage_backend.c index 32a8d65..58d8642 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -338,7 +338,7 @@ int virStorageBackendRunProgRegex(virConnectPtr conn, while (fgets(line, sizeof(line), list) != NULL) { /* Strip trailing newline */ int len = strlen(line); - if (line[len-1] == '\n') + if (len && line[len-1] == '\n') line[len-1] = '\0'; for (i = 0 ; i <= maxReg && i < nregex ; i++) { -- 1.5.4.rc4.1.g1895

3 3

[Libvir] PATCH: Fix qemu-dm path in capabilities info
by Daniel P. Berrange 21 Jan '08

21 Jan '08

The current Xen capabilities XML data chooses between /usr/lib/xen/bin/qemu-dm and /usr/lib64/xen/bin/qemu-dm based on whether the guest arch is 64 bit. This is incorrect because the path is actually determined based on the host OS. Only x86_64 uses /usr/lib64, all other arches just use /usr/lib - even on ia64. So this patch changes the logic so that /usr/lib64 is only used if we see an x86_64 guest present - this implies host is x86_64 and that all guests whether 32 or 64bit shoudl use /usr/lib64 Dan. diff -r 38f2abccef9b src/xen_internal.c --- a/src/xen_internal.c Sun Jan 20 11:28:37 2008 -0500 +++ b/src/xen_internal.c Sun Jan 20 12:36:23 2008 -0500 @@ -2186,6 +2186,7 @@ xenHypervisorMakeCapabilitiesXML(virConn int ia64_be; } guest_archs[32]; int nr_guest_archs = 0; + int lib64 = 0; virBufferPtr xml; char *xml_str; @@ -2261,6 +2262,7 @@ xenHypervisorMakeCapabilitiesXML(virConn else if (strncmp (&token[subs[2].rm_so], "x86_64", 6) == 0) { model = "x86_64"; bits = 64; + lib64 = 1; } else if (strncmp (&token[subs[2].rm_so], "ia64", 4) == 0) { model = "ia64"; @@ -2376,7 +2378,7 @@ xenHypervisorMakeCapabilitiesXML(virConn <machine>pc</machine>\n\ <machine>isapc</machine>\n\ <loader>/usr/lib/xen/boot/hvmloader</loader>\n", - guest_archs[i].bits == 64 ? "64" : ""); + lib64 ? "64" : ""); if (r == -1) goto vir_buffer_failed; } r = virBufferAdd (xml, -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

2 2

[Libvir] Dom0 and entitlement
by Katti, Vadiraj (STSD-Openview) 21 Jan '08

21 Jan '08

Hi, I've some questions related to xen. Can anybody help me regarding the following questions. 1. Is dom0 a logical system(like any other vm's that are configured on it) ? If yes, then all xm commands should be applicable to it (ex : vcpu-set) . 2. /proc on dom0 reflects the physical or logical utilization. 3. Is there any entitelment concept in xen for cpu and memory? What is cpu weight used for and how is set. 4. virt-manager shows physical or logical utilization (cpu, memory) 5. How do i get the IP address of each of the guests from dom0 ? 6. What are the interfaces in domU to tell me that its a guest (or dom0) and also the type of virtualization ( eg. XEN/ VMWARE etc ) Thanks, Vadiraj

2 1

[Libvir] Question about building libvirt on Windows
by Atsushi SAKAI 21 Jan '08

21 Jan '08

Hi, Rich I try to build on current libvirt on Windows. But failed in attached compilation message. I would be appreciate If you have any suggestions. My Instruction is follows. build cygwinenvironment. Install gettext-devel, gnutls, gnutls-devel additionally. install sunrpc-4.0-4 on / compile libvirt(current) and fails and get attached message. I have not added any additional patches to libvirt. Thanks Atsushi SAKAI

2 1

[Libvir] Windows support document seems little bit old.
by Atsushi SAKAI 21 Jan '08

21 Jan '08

Hi, Rich I am trying to build libvirt on Windows for checking Xen domain operation. I am doing this based on following instruction. http://libvirt.org/windows.html Anyway Your document seems old. since the suggest patch is already applied to libvirt itself. Is there any plan to update it? Thanks Atsushi SAKAI

2 1