[libvirt] virsh --connect not working
I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Thanks in advance, Hiren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh --connect not working
On Thu, Sep 27, 2012 at 11:15 PM, hiren panchasara hiren.panchas...@gmail.com wrote: I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Just wanted to add that I have qemu installed. Not sure how to connect that with virsh. Thanks, Hiren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] python: cleanup vcpu related binding APIs
On 09/27/2012 09:51 PM, Michal Privoznik wrote: On 26.09.2012 19:33, Guannan Ren wrote: libvirt_virDomainGetVcpus: add error handling, return -1 instead of None libvirt_virDomainPinVcpu and libvirt_virDomainPinVcpuFlags: make use of libvirt_boolUnwrap Set bitmap according to these values which are contained in given argument of vcpu tuple and turn off these bit corresponding to missing vcpus in argument tuple The original way ignored the error info from PyTuple_GetItem if index is out of range. IndexError: tuple index out of range The error message will only be raised on next command in interactive mode. --- python/libvirt-override.c | 171 +- 1 file changed, 123 insertions(+), 48 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 25f9d3f..b69f5cf 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1333,9 +1333,11 @@ cleanup: static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *pyretval = NULL, *pycpuinfo = NULL, *pycpumap = NULL; +PyObject *error = NULL; You are not setting this variable before every 'goto cleanup;' and I think it should be done. Or is it okay to return NULL? we set error variable in two places. error = PyErr_NoMemory(); error = VIR_PY_INT_FAIL; For the other places before goto cleanup, we don't need to set specific error because the cpython function did it already. what left to me is to return NULL to show these error messages to user. In common, if cpython function(e.g PyTuple_New()) reports errors, we just return NULL. No. I know python bindings code are mess but this won't make it any better. First of all: if (cond1) code_block; else if (cond2) the_very_same_codeblock; can be joined like this: if (cond1 || cond2) code_block; Thanks I will change to this. Second - drop the do { } while(0) and use a label instead: if (cond1 || cond2) goto label; without do () while, we have to have two labels I think it will be a little messy. I tried the following before, it is ok to walk back. if (cond1 || cond2) goto label; continue; label: Py_DECREF(info); .. goto cleanup This label can in this special case be at the end of the parent for loop; However, there needs to be 'continue' just before the label so the for loop doesn't execute the label itself. For the following, I will change according to your comments. Thanks for this review. :) Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Move most of qemuProcessKill into virProcessKillPainfully
On Thu, Sep 27, 2012 at 04:31:10PM -0400, Laine Stump wrote: On 09/26/2012 10:44 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com In the cgroups APIs we have a virCgroupKillPainfully function which does the loop sending SIGTERM, then SIGKILL and waiting for the process to exit. There is similar functionality for simple processes in qemuProcessKill, but it is tangled with the QEMU code. Untangle it to provide a virProcessKillPainfuly function --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 8 ++--- src/qemu/qemu_process.c | 79 src/util/virprocess.c| 57 ++ src/util/virprocess.h| 2 ++ 5 files changed, 76 insertions(+), 71 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4635a4d..dab607a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1708,6 +1708,7 @@ virPidFileDeletePath; # virprocess.h virProcessAbort; virProcessKill; +virProcessKillPainfully; virProcessTranslateStatus; virProcessWait; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7ac53ac..22fef7a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2006,13 +2006,11 @@ qemuDomainDestroyFlags(virDomainPtr dom, * it now means the job will be released */ if (flags VIR_DOMAIN_DESTROY_GRACEFUL) { -if (qemuProcessKill(driver, vm, 0) 0) { -virReportError(VIR_ERR_OPERATION_FAILED, %s, - _(failed to kill qemu process with SIGTERM)); +if (qemuProcessKill(driver, vm, 0) 0) goto cleanup; -} } else { -ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE)); +if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) 0) +goto cleanup; } /* We need to prevent monitor EOF callback from doing our work (and sending diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3cd30af..70b72af 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3877,9 +3877,7 @@ int qemuProcessKill(struct qemud_driver *driver, virDomainObjPtr vm, unsigned int flags) { -int i, ret = -1; -const char *signame = TERM; -bool driver_unlocked = false; +int ret; VIR_DEBUG(vm=%s pid=%d flags=%x, vm-def-name, vm-pid, flags); @@ -3891,78 +3889,27 @@ qemuProcessKill(struct qemud_driver *driver, } } -/* This loop sends SIGTERM (or SIGKILL if flags has - * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT), - * then waits a few iterations (10 seconds) to see if it dies. If - * the qemu process still hasn't exited, and - * VIR_QEMU_PROCESS_KILL_FORCE is requested, a SIGKILL will then - * be sent, and qemuProcessKill will wait up to 5 seconds more for - * the process to exit before returning. Note that the FORCE mode - * could result in lost data in the guest, so it should only be - * used if the guest is hung and can't be destroyed in any other - * manner. - */ -for (i = 0 ; i 75; i++) { -int signum; -if (i == 0) { -if ((flags VIR_QEMU_PROCESS_KILL_FORCE) -(flags VIR_QEMU_PROCESS_KILL_NOWAIT)) { -signum = SIGKILL; /* kill it immediately */ -signame=KILL; -} else { -signum = SIGTERM; /* kindly suggest it should exit */ -} -} else if ((i == 50) (flags VIR_QEMU_PROCESS_KILL_FORCE)) { -VIR_WARN(Timed out waiting after SIG%s to process %d, - sending SIGKILL, signame, vm-pid); -signum = SIGKILL; /* kill it after a grace period */ -signame=KILL; -} else { -signum = 0; /* Just check for existence */ -} - -if (virProcessKill(vm-pid, signum) 0) { -if (errno != ESRCH) { -char ebuf[1024]; -VIR_WARN(Failed to terminate process %d with SIG%s: %s, - vm-pid, signame, - virStrerror(errno, ebuf, sizeof(ebuf))); -goto cleanup; -} -ret = 0; -goto cleanup; /* process is dead */ -} +if ((flags VIR_QEMU_PROCESS_KILL_NOWAIT)) { +virProcessKill(vm-pid, + (flags VIR_QEMU_PROCESS_KILL_FORCE) ? + SIGKILL : SIGTERM); +return 0; +} -if (i == 0 (flags VIR_QEMU_PROCESS_KILL_NOWAIT)) { -ret = 0; -goto cleanup; -} +if (driver) +
Re: [libvirt] [PATCH 2/9] s/long long/size_t/ for file line numbers in logging code
On Thu, Sep 27, 2012 at 01:05:55PM -0600, Eric Blake wrote: On 09/27/2012 10:44 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The __LINE__ macro value is specified to fit in the size_t Not quite accurate. C99 merely says: 6.10.4 p3: ... a line number as specified by the digit sequence (interpreted as a decimal integer). The digit sequence shall not specify zero, nor a number greater than 2147483647. 6.10.8 p1: _ _LINE_ _ The presumed line number (within the current source file) of the current source line (an integer constant). So in fact, __LINE__ is guaranteed to fit within a 32-bit signed integer, and you could s/size_t/int/ with no loss in functionality since we don't port to 16-bit 'int' platforms. That said, I'm not too fussed with things; size_t is definitely better than 'long long', so I'm okay even if you don't further relax to 'int'. Based on discussion in the later patches, I'm happy to change this to just 'int'. Still surprised it wasn't actually size_t in the standard, but oh well. 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 7/9] Add systemd journal support
On Thu, Sep 27, 2012 at 02:51:35PM -0600, Eric Blake wrote: On 09/27/2012 10:44 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Add support for logging to the systemd journal, using its simple client library. The benefit over syslog is that it accepts structured log data, so the journald can store individual items like code file/line/func separately from the string message. Tools which require structured log data can then query the journal to extract exactly what they desire without resorting to string parsing While systemd provides a simple client library for logging, it is more convenient for libvirt to directly write its own client code. This lets us build up the iovec's on the stack, avoiding the need to alloc memory when writing log messages. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- cfg.mk | 2 +- src/util/logging.c | 177 + src/util/logging.h | 1 + 3 files changed, 179 insertions(+), 1 deletion(-) I'll need a v2 on this one. Fails to compile: util/logging.c: In function 'virLogOutputToJournald': util/logging.c:1124:84: error: ordered comparison of pointer with integer zero [-Werror=extra] util/logging.c:1124:18: error: ignoring return value of 'virStrcpy', declared with attribute warn_unused_result [-Werror=unused-result] cc1: all warnings being treated as errors diff --git a/cfg.mk b/cfg.mk index bbfd4a2..4482d70 100644 --- a/cfg.mk +++ b/cfg.mk @@ -771,7 +771,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(bootstrap.conf$$|src/util/util\.c$$|examples/domain-events/events-c/event-test\.c$$) exclude_file_name_regexp--sc_prohibit_close = \ - (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c)$$) + (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|src/util/logging\.c)$$) That's a bit of a heavy hammer, just to avoid a recursively-logged close(). I'd rather omit this and use VIR_LOG_CLOSE(). Ahhh, I didn't notice VIR_LOG_CLOSE. I did indeed hit a recursively logging close :-) + + +# define IOVEC_SET_STRING(iov, str) \ +do { \ +struct iovec *_i = (iov); \ +_i-iov_base = (char*)str; \ +_i-iov_len = strlen(str); \ +} while (0) + +# define IOVEC_SET_INT(iov, buf, fmt, val) \ +do {\ +struct iovec *_i = (iov); \ +snprintf(buf, sizeof(buf), fmt, val); \ Technically, snprintf is not async-signal safe, which kind of goes against the idea of using it in our logging functions. I'd much rather see us do a hand-rolled conversion of int to %d or %zu (which appear to be the only formats you ever pass here). Ok, should be easy enough, although I notice that the code we use for generating the timestamp string also uses snprintf(), so we should fix that sometime too. + * encode the string length, since we can't + * rely on the newline for the field separator + */ +IOVEC_SET_STRING(iov[niov++], MESSAGE\n); +nstr = htole64(strlen(rawstr)); htole64() is a glibc extension which does not exist on mingw, and has not (yet) been ported to gnulib. Then again, this code is inside #ifdef HAVE_SYSLOG_H, so you can probably get away with it (although I'm not sure whether all platforms that provide syslog.h happen to also provide htole64(), I at least know that it will exclude mingw). I think I'll #ifdef __linux__ the whole block just to be sure, since systemd is only ever going to be Linux based +/* Message was too large, so dump to temporary file + * and pass an FD to the journal + */ + +if ((buffd = mkostemp(path, O_CLOEXEC|O_RDWR)) 0) Is mkostemp async-signal safe? But if it isn't, I don't know how else to generate a safe file name. Maybe we create ourselves a safe temporary directory at process start where we don't care about the async safety issues, and then in this function, we track a static counter that we increment each time we create a new file within that directory. Yep, we never need multiple concurrent temp files, so I can just create one when we initialize logging, and hold open its file descriptor. +return; + +if (unlink(path) 0) +goto cleanup; + +if (writev(buffd, iov, niov) 0) Again, mingw and gnulib lacks writev(), but this function isn't compiled on mingw, so we're safe for now. +goto cleanup; + +mh.msg_iov = NULL; +mh.msg_iovlen = 0; + +memset(control, 0, sizeof(control)); +mh.msg_control = control; +mh.msg_controllen = sizeof(control); + +cmsg = CMSG_FIRSTHDR(mh); +cmsg-cmsg_level = SOL_SOCKET; +cmsg-cmsg_type = SCM_RIGHTS; +cmsg-cmsg_len =
Re: [libvirt] [PATCH 0/9] Use the systemd journal for logging
On Thu, Sep 27, 2012 at 07:31:41PM -0500, Doug Goldstein wrote: On Thu, Sep 27, 2012 at 11:44 AM, Daniel P. Berrange berra...@redhat.com wrote: The systemd journal provides a highly efficient way for apps to generate structured log messages. Since systemd will be used by default on future Fedora/RHEL distros (and likely many others), we should take advantage of the journal out of the box for improved logging capabilities. Amongst other things, this allows very easy browsing of the libvirt logs after the fact, filtering on filename / line number / function name and more. This is a big benefit to debugging, compared to current case where we need to decide the filters when starting libvirtd. For example it becomes possible to configure libvirt to include all QEMU logging, and then later ask the journal for only messages related to the QEMU json monitor. -- Someone else posted a patchset for structured logging via lumberjack which would support systemd as well as the other syslog daemons in an abstract way without directly tying the project to systemd? That seems like a better option IMHO. So I would vote for a NACK for the series. First off, we're not tieing the project to systemd, because this is an entirely optional feature. If you read my response to the lumberjack posting, you'll see that there are a number of serious architectural flaws in the design of lumberjack for use as a structured logging system. Unless they radically change the design, which appears unlikely, it is not going to be suitable as a logging impl for libvirt (or arguably any system daemon) to use. 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] virsh --connect not working
On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote: I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Try collecting some debugging output using LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system It should give a better idea why it failed. 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 v3] Fix dnsmasq/radvd on bridges not being able to be bound to IPv6 address on recent kernels
Le jeudi 27 septembre 2012 à 11:18 -0600, Eric Blake a écrit : ACK with those changes, so I fixed it and pushed, along with your .mailmap update. Thanks a lot! -- Benjamin Cama benjamin.c...@telecom-bretagne.eu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] python: return error if PyObject obj is NULL for unwrapper helper functions
On 09/27/2012 09:51 PM, Michal Privoznik wrote: On 26.09.2012 19:33, Guannan Ren wrote: The result is indeterminate for NULL argument to python functions as follows. It's better to return negative value in these situations. PyObject_IsTrue will segfault if the argument is NULL PyFloat_AsDouble(NULL) is -1.00 PyLong_AsUnsignedLongLong(NULL) is 0.00 --- python/typewrappers.c | 43 +-- 1 file changed, 41 insertions(+), 2 deletions(-) ACK Michal Thanks and pushed Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] python: cleanup vcpu related binding APIs
libvirt_virDomainGetVcpus: add error handling, return -1 instead of None libvirt_virDomainPinVcpu and libvirt_virDomainPinVcpuFlags: check the type of argument make use of libvirt_boolUnwrap Set bitmap according to these values which are contained in given argument of vcpu tuple and turn off these bit corresponding to missing vcpus in argument tuple The original way ignored the error info from PyTuple_GetItem if index is out of range. IndexError: tuple index out of range The error message will only be raised on next command in interactive mode. --- python/libvirt-override.c | 163 ++ 1 file changed, 121 insertions(+), 42 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 25f9d3f..81099b1 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1333,9 +1333,11 @@ cleanup: static PyObject * libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, - PyObject *args) { + PyObject *args) +{ virDomainPtr domain; PyObject *pyobj_domain, *pyretval = NULL, *pycpuinfo = NULL, *pycpumap = NULL; +PyObject *error = NULL; virNodeInfo nodeinfo; virDomainInfo dominfo; virVcpuInfoPtr cpuinfo = NULL; @@ -1352,29 +1354,33 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, i_retval = virNodeGetInfo(virDomainGetConnect(domain), nodeinfo); LIBVIRT_END_ALLOW_THREADS; if (i_retval 0) -return VIR_PY_NONE; +return VIR_PY_INT_FAIL; LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetInfo(domain, dominfo); LIBVIRT_END_ALLOW_THREADS; if (i_retval 0) -return VIR_PY_NONE; +return VIR_PY_INT_FAIL; if (VIR_ALLOC_N(cpuinfo, dominfo.nrVirtCpu) 0) -return VIR_PY_NONE; +return PyErr_NoMemory(); cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo)); if (xalloc_oversized(dominfo.nrVirtCpu, cpumaplen) || -VIR_ALLOC_N(cpumap, dominfo.nrVirtCpu * cpumaplen) 0) +VIR_ALLOC_N(cpumap, dominfo.nrVirtCpu * cpumaplen) 0) { +error = PyErr_NoMemory(); goto cleanup; +} LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetVcpus(domain, cpuinfo, dominfo.nrVirtCpu, cpumap, cpumaplen); LIBVIRT_END_ALLOW_THREADS; -if (i_retval 0) +if (i_retval 0) { +error = VIR_PY_INT_FAIL; goto cleanup; +} /* convert to a Python tuple of long objects */ if ((pyretval = PyTuple_New(2)) == NULL) @@ -1386,13 +1392,35 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, for (i = 0 ; i dominfo.nrVirtCpu ; i++) { PyObject *info = PyTuple_New(4); +PyObject *item = NULL; + 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); + +if ((item = PyInt_FromLong((long)cpuinfo[i].number)) == NULL || +PyTuple_SetItem(info, 0, item) 0) +goto itemError; + +if ((item = PyInt_FromLong((long)cpuinfo[i].state)) == NULL || +PyTuple_SetItem(info, 1, item) 0) +goto itemError; + +if ((item = PyLong_FromLongLong((long long)cpuinfo[i].cpuTime)) == NULL || +PyTuple_SetItem(info, 2, item) 0) +goto itemError; + +if ((item = PyInt_FromLong((long)cpuinfo[i].cpu)) == NULL || +PyTuple_SetItem(info, 3, item) 0) +goto itemError; + +if (PyList_SetItem(pycpuinfo, i, info) 0) +goto itemError; + +continue; +itemError: +Py_DECREF(info); +Py_XDECREF(item); +goto cleanup; } for (i = 0 ; i dominfo.nrVirtCpu ; i++) { PyObject *info = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo)); @@ -1400,36 +1428,48 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, 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))); +PyObject *item = NULL; +if ((item = PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, i, j))) == NULL || +PyTuple_SetItem(info, j, item) 0) { +Py_DECREF(info); +Py_XDECREF(item); +goto cleanup; +} +} +if (PyList_SetItem(pycpumap, i, info) 0) { +Py_DECREF(info); +goto cleanup; } -PyList_SetItem(pycpumap, i,
Re: [libvirt] [PATCH 7/9] Add systemd journal support
On Thu, Sep 27, 2012 at 02:51:35PM -0600, Eric Blake wrote: On 09/27/2012 10:44 AM, Daniel P. Berrange wrote: +/* Message was too large, so dump to temporary file + * and pass an FD to the journal + */ + +if ((buffd = mkostemp(path, O_CLOEXEC|O_RDWR)) 0) Is mkostemp async-signal safe? But if it isn't, I don't know how else to generate a safe file name. Maybe we create ourselves a safe temporary directory at process start where we don't care about the async safety issues, and then in this function, we track a static counter that we increment each time we create a new file within that directory. I've looked the glibc source and the only functions they use are open() and gettimeofday(), and the latter isn't actually used on most architectures, instead it uses inline asm to read a CPU timesource like the TSC. So IMHO, since this will be protected by a #ifdef __linux__ we will be safe in using 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
Re: [libvirt] [PATCH] fix-up an implicit build dead-loop in cfg.mk
On 09/27/2012 09:51 PM, liguang wrote: if gnulib submodule happened to be dirty, build process will fall into '_autogen' target trap in cfg.mk recursively, so break this dead-loop. Yes, I've run into this before; thanks for trying to tackle it. Signed-off-by: liguang lig.f...@cn.fujitsu.com --- cfg.mk |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/cfg.mk b/cfg.mk index bbfd4a2..92966d5 100644 --- a/cfg.mk +++ b/cfg.mk @@ -699,7 +699,7 @@ ifeq (0,$(MAKELEVEL)) test $$stamp = $$actual; echo $$?) _clean_requested = $(filter %clean,$(MAKECMDGOALS)) ifeq (1,$(_update_required)$(_clean_requested)) -$(info INFO: gnulib update required; running ./autogen.sh first) +$(error ERR: gnulib update required; running ./autogen.sh first) However, this is not the right fix - it will error out even when .gnulib is not dirty, but just out of date. I'll spend some time on this getting it right today. -- 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] python: keep consistent handling of Python integer conversion
libvirt_ulonglongUnwrap requires the integer type of python obj. But libvirt_longlongUnwrap still could handle python obj of Pyfloat_type which causes the float value to be rounded up to an integer. For example dom.setSchedulerParameters({'vcpu_quota': 0.88}) 0 libvirt_longlongUnwrap treats 0.88 as a valid value 0 However dom.setSchedulerParameters({'cpu_shares': 1000.22}) libvirt_ulonglongUnwrap will throw out an error TypeError: an integer is required The patch make this consistent. --- python/typewrappers.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/python/typewrappers.c b/python/typewrappers.c index 7580689..d633603 100644 --- a/python/typewrappers.c +++ b/python/typewrappers.c @@ -220,7 +220,7 @@ libvirt_ulongUnwrap(PyObject *obj, unsigned long *val) int libvirt_longlongUnwrap(PyObject *obj, long long *val) { -long long llong_val; +long long llong_val = -1; if (!obj) { PyErr_SetString(PyExc_TypeError, unexpected type); @@ -230,7 +230,11 @@ libvirt_longlongUnwrap(PyObject *obj, long long *val) /* If obj is of PyInt_Type, PyLong_AsLongLong * will call PyInt_AsLong() to handle it automatically. */ -llong_val = PyLong_AsLongLong(obj); +if (PyInt_Check(obj) || PyLong_Check(obj)) +llong_val = PyLong_AsLongLong(obj); +else +PyErr_SetString(PyExc_TypeError, an integer is required); + if ((llong_val == -1) PyErr_Occurred()) return -1; -- 1.7.11.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] Add systemd journal support
On 09/28/2012 02:08 AM, Daniel P. Berrange wrote: +static int virLogAddOutputToJournald(int priority) +{ +if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) 0) +return -1; +if (virSetInherit(journalfd, false) 0) { Why not use SOCK_DGRAM | SOCK_CLOEXEC in the socket() call? It wasn't clear to me whether GNULIB provides compat for SOCK_CLOEXEC or not. Even though we're only Linux, older glibc won't provide that I looked; gnulib guarantees SOCK_CLOEXEC for accept4(), but has not yet ported it to socket(). I guess we stick with two calls for now. @@ -1114,6 +1285,12 @@ virLogParseOutputs(const char *outputs) count++; VIR_FREE(name); VIR_FREE(abspath); +} else if (STREQLEN(cur, journald, 8)) { +cur += 8; +#if HAVE_SYSLOG_H Do we really want to silently parse and ignore 'journald' on systems that lack syslog.h? I just followed the same pattern that we used with the syslog parsing. Reading patch 8/9 makes me concur - there, we try a log style, then if is it silently ignored, virLogGetNbOutputs() returns 0, so we try the next log style. This is fine after all. -- 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 7/9] Add systemd journal support
On 09/28/2012 04:09 AM, Daniel P. Berrange wrote: On Thu, Sep 27, 2012 at 02:51:35PM -0600, Eric Blake wrote: On 09/27/2012 10:44 AM, Daniel P. Berrange wrote: +/* Message was too large, so dump to temporary file + * and pass an FD to the journal + */ + +if ((buffd = mkostemp(path, O_CLOEXEC|O_RDWR)) 0) Is mkostemp async-signal safe? But if it isn't, I don't know how else to generate a safe file name. Maybe we create ourselves a safe temporary directory at process start where we don't care about the async safety issues, and then in this function, we track a static counter that we increment each time we create a new file within that directory. I've looked the glibc source and the only functions they use are open() and gettimeofday(), and the latter isn't actually used on most architectures, instead it uses inline asm to read a CPU timesource like the TSC. So IMHO, since this will be protected by a #ifdef __linux__ we will be safe in using it. Sure, but add a big fat comment explaining our choice, so that people reading the code later remember our discussion :) -- 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 v2] Add systemd journal support
From: Daniel P. Berrange berra...@redhat.com Add support for logging to the systemd journal, using its simple client library. The benefit over syslog is that it accepts structured log data, so the journald can store individual items like code file/line/func separately from the string message. Tools which require structured log data can then query the journal to extract exactly what they desire without resorting to string parsing While systemd provides a simple client library for logging, it is more convenient for libvirt to directly write its own client code. This lets us build up the iovec's on the stack, avoiding the need to alloc memory when writing log messages. Changed in v2: - Add virFormatIntDecimal instead of using snprintf - Add comment about mkostemp - Fix declaration of linestr var to use size of linenr - Wrap in #ifdef __linux__ Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms | 1 + src/util/logging.c | 184 +++ src/util/logging.h | 1 + src/util/util.c | 30 src/util/util.h | 5 +- 5 files changed, 220 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dab607a..eebc52a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1233,6 +1233,7 @@ virFileUnlock; virFileWaitForDevices; virFileWriteStr; virFindFileInPath; +virFormatIntDecimal; virGetGroupID; virGetGroupName; virGetHostname; diff --git a/src/util/logging.c b/src/util/logging.c index cdd94fd..9e44880 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -35,6 +35,10 @@ #if HAVE_SYSLOG_H # include syslog.h #endif +#include sys/socket.h +#if HAVE_SYS_UN_H +# include sys/un.h +#endif #include virterror_internal.h #include logging.h @@ -44,6 +48,7 @@ #include threads.h #include virfile.h #include virtime.h +#include intprops.h #define VIR_FROM_THIS VIR_FROM_NONE @@ -146,6 +151,8 @@ virLogOutputString(virLogDestination ldest) return syslog; case VIR_LOG_TO_FILE: return file; +case VIR_LOG_TO_JOURNALD: +return journald; } return unknown; } @@ -1020,6 +1027,177 @@ virLogAddOutputToSyslog(virLogPriority priority, } return 0; } + + +# ifdef __linux__ +# define IOVEC_SET_STRING(iov, str) \ +do { \ +struct iovec *_i = (iov); \ +_i-iov_base = (char*)str; \ +_i-iov_len = strlen(str); \ +} while (0) + +# define IOVEC_SET_INT(iov, buf, val) \ +do {\ +struct iovec *_i = (iov); \ +_i-iov_base = virFormatIntDecimal(buf, sizeof(buf), val); \ +_i-iov_len = strlen(buf); \ +} while (0) + +static int journalfd = -1; + +static void +virLogOutputToJournald(virLogSource source, + virLogPriority priority, + const char *filename, + int linenr, + const char *funcname, + const char *timestamp ATTRIBUTE_UNUSED, + unsigned int flags, + const char *rawstr, + const char *str ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ +virCheckFlags(VIR_LOG_STACK_TRACE,); +int buffd = -1; +size_t niov = 0; +struct msghdr mh; +struct sockaddr_un sa; +union { +struct cmsghdr cmsghdr; +uint8_t buf[CMSG_SPACE(sizeof(int))]; +} control; +struct cmsghdr *cmsg; +/* We use /dev/shm instead of /tmp here, since we want this to + * be a tmpfs, and one that is available from early boot on + * and where unprivileged users can create files. */ +char path[] = /dev/shm/journal.XX; +char priostr[INT_BUFSIZE_BOUND(priority)]; +char linestr[INT_BUFSIZE_BOUND(linenr)]; + +/* First message takes upto 4 iovecs, and each + * other field needs 3, assuming they don't have + * newlines in them + */ +# define IOV_SIZE (4 + (5 * 3)) +struct iovec iov[IOV_SIZE]; + +if (strchr(rawstr, '\n')) { +uint64_t nstr; +/* If 'str' containes a newline, then we must + * encode the string length, since we can't + * rely on the newline for the field separator + */ +IOVEC_SET_STRING(iov[niov++], MESSAGE\n); +nstr = htole64(strlen(rawstr)); +iov[niov].iov_base = (char*)nstr; +iov[niov].iov_len = sizeof(nstr); +niov++; +} else { +IOVEC_SET_STRING(iov[niov++], MESSAGE=); +} +IOVEC_SET_STRING(iov[niov++], rawstr); +IOVEC_SET_STRING(iov[niov++], \n); + +IOVEC_SET_STRING(iov[niov++], PRIORITY=); +IOVEC_SET_INT(iov[niov++], priostr,
Re: [libvirt] [PATCH v3 20/20] Add support for detecting capablities using QMP commands
On Thu, Sep 27, 2012 at 03:00:14PM +0200, Jiri Denemark wrote: On Tue, Sep 25, 2012 at 19:00:13 +0100, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Start a QEMU process using $QEMU -S -no-user-config -nodefconfig -nodefaults \ -nographic -M none -qmp stdio -nodefconfig should not ever be used if QEMU supports -no-user-config. The reason is that -nodefconfig disables loading of all files even those that reside somewhere in /usr/share and may contain required data, such as CPU definitions (although they were moved back to the code recently) or machine type definitions if they ever implement the ideas to separate them from the code. Hmm, yes, I forgot about that. @@ -1282,6 +1285,7 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = { }; static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = { +{ rombar, QEMU_CAPS_PCI_ROMBAR }, { configfd, QEMU_CAPS_PCI_CONFIGFD }, { bootindex, QEMU_CAPS_PCI_BOOTINDEX }, }; @@ -1857,6 +1861,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps, qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC); else if (STREQ(name, dump-guest-memory)) qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY); +else if (STREQ(name, query-spice)) +qemuCapsSet(caps, QEMU_CAPS_SPICE); +else if (STREQ(name, query-kvm)) +qemuCapsSet(caps, QEMU_CAPS_KVM); VIR_FREE(name); } VIR_FREE(commands); Hmm, looks like these two hunks should rather go into the previous patch, shouldn't they? No, the previous patch was just refactoring existing code. The existing approach to detecting spice/kvm was to use -help parsing. Only with the new QMP code in this patch, do we now detect it via the QMP command list @@ -1979,18 +2078,252 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) * understands the 0.13.0+ notion of -device driver,. */ if (qemuCapsGet(caps, QEMU_CAPS_DEVICE) strstr(help, -device driver,?) -qemuCapsExtractDeviceStr(binary, caps) 0) -goto error; +qemuCapsExtractDeviceStr(caps-binary, caps) 0) +goto cleanup; if (qemuCapsProbeCPUModels(caps) 0) -goto error; +goto cleanup; if (qemuCapsProbeMachineTypes(caps) 0) -goto error; +goto cleanup; + +ret = 0; +cleanup: +virCommandFree(cmd); +return ret; +} I think you actually didn't want to remove VIR_FREE(help); from the cleanup section. Yeah, bug. +static int +qemuCapsInitQMP(qemuCapsPtr caps) +{ +int ret = -1; +virCommandPtr cmd = NULL; +virDomainObjPtr vm = NULL; +int socks[2] = { -1, -1 }; +qemuMonitorPtr mon = NULL; +int major, minor, micro; +char *package; + +VIR_DEBUG(caps=%p, caps); + +if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks) 0) { +virReportSystemError(errno, %s, + _(unable to create socket pair)); +goto cleanup; +} + +if (!(vm = virDomainObjNew(NULL))) +goto cleanup; Heh, AFAICT, vm in qemuMonitorOpenFD is only used to fill in mon-vm so that the VM pointer can be passed to various callbacks when something asynchronous happens in the monitor. Looks like we can just safely pass NULL to qemuMonitorOpenFD() instead. There is one place in the monitor which uses it, but we can avoid it. + +cmd = virCommandNewArgList(caps-binary, + -S, + -no-user-config, + -nodefconfig, Remove the -nodefconfig parameter, since this code requires at least qemu-1.2 and thus it will either support -no-user-config or be unusable anyway. + -nodefaults, + -nographic, + -M, none, + -qmp, stdio, + NULL); +virCommandAddEnvPassCommon(cmd); +virCommandSetInputFD(cmd, socks[1]); +virCommandSetOutputFD(cmd, socks[1]); +virCommandClearCaps(cmd); + +if (virCommandRunAsync(cmd, NULL) 0) +goto cleanup; However, if qemu is not new enough to support -no-user-config, virCommand will fail and this would consider it a fatal error instead of falling back to help parsing. Actually not, because we're running async here, so we don't detect the failed QEMU at this point, only later in this method. There is a problem here though - my current code will cause log messages to be generated when hitting old QEMU which is undesirable. So I'm changing this to use virCommandRun and -daemonize QEMU, so we can detect exit status for old QEMU without polluting the log. Enough changes that I'll re-post this. Daniel -- |: http://berrange.com
Re: [libvirt] [PATCH v2] Add systemd journal support
On 09/28/2012 07:08 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Add support for logging to the systemd journal, using its simple client library. The benefit over syslog is that it accepts structured log data, so the journald can store individual items like code file/line/func separately from the string message. Tools which require structured log data can then query the journal to extract exactly what they desire without resorting to string parsing While systemd provides a simple client library for logging, it is more convenient for libvirt to directly write its own client code. This lets us build up the iovec's on the stack, avoiding the need to alloc memory when writing log messages. Changed in v2: - Add virFormatIntDecimal instead of using snprintf - Add comment about mkostemp - Fix declaration of linestr var to use size of linenr - Wrap in #ifdef __linux__ ACK with one nit: +# ifdef __linux__ +static void +virLogOutputToJournald(virLogSource source, +# endif /* __linux__ */ #endif /* HAVE_SYSLOG_H */ #define IS_SPACE(cur) \ @@ -1114,6 +1292,12 @@ virLogParseOutputs(const char *outputs) count++; VIR_FREE(name); VIR_FREE(abspath); +} else if (STREQLEN(cur, journald, 8)) { +cur += 8; +#if HAVE_SYSLOG_H +if (virLogAddOutputToJournald(prio) == 0) This will fail to compile on non-Linux systems that have syslog.h. Make the condition: #if HAVE_SYSLOG_H defined __linux__ -- 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] Memory free in libvirt JNA
Hi Claudio, sorry for the delay, I need to focuse one something else ATM ! On Mon, Sep 24, 2012 at 04:11:19PM +0200, Claudio Bley wrote: At Wed, 12 Sep 2012 20:23:28 +0800, Daniel Veillard wrote: I think we must provide the free functions for all the memory allocated by libvirt. Okay, can you work on making a patch ? To be honnest I'm very unlikely to have time for this in the short term, I did notice the same thing (but was on vacation for quite some time). I already have a fix for virDomainGetSchedulerType in my local repository (see below). okay i see I'm using virFree to free the allocated memory which adds another level of indirection (PointerByReference). This is because JNA does not provide access to the C free() function by itself, AFAICS. -- 8 --- From cb58e9538afbfd7ce09c5c2a93e403a809e77113 Mon Sep 17 00:00:00 2001 From: Claudio Bley cb...@av-test.de Date: Fri, 24 Aug 2012 11:09:49 +0200 Subject: [PATCH] Fix memory leak for virDomainGetSchedulerType. The string returned by virDomainGetSchedulerType must be free'd explicitly. JNA docs say that we have to use a Pointer as a return type and free it ourselves. So, simply use virFree for that matter. --- src/main/java/org/libvirt/Domain.java | 13 + src/main/java/org/libvirt/jna/Libvirt.java | 6 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/libvirt/Domain.java b/src/main/java/org/libvirt/Domain.java index 1f15ee8..fc1f665 100644 --- a/src/main/java/org/libvirt/Domain.java +++ b/src/main/java/org/libvirt/Domain.java @@ -14,7 +14,9 @@ import org.libvirt.jna.virVcpuInfo; import com.sun.jna.Native; import com.sun.jna.NativeLong; +import com.sun.jna.Pointer; import com.sun.jna.ptr.IntByReference; +import com.sun.jna.ptr.PointerByReference; /** * A virtual machine defined within libvirt. @@ -443,9 +445,11 @@ public class Domain { public SchedParameter[] getSchedulerParameters() throws LibvirtException { IntByReference nParams = new IntByReference(); SchedParameter[] returnValue = new SchedParameter[0]; -String scheduler = libvirt.virDomainGetSchedulerType(VDP, nParams); +Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams); processError(); -if (scheduler != null) { +if (pScheduler != null) { +String scheduler = pScheduler.getString(0); +libvirt.virFree(new PointerByReference(pScheduler)); virSchedParameter[] nativeParams = new virSchedParameter[nParams.getValue()]; returnValue = new SchedParameter[nParams.getValue()]; libvirt.virDomainGetSchedulerParameters(VDP, nativeParams, nParams); @@ -470,10 +474,11 @@ public class Domain { */ public String[] getSchedulerType() throws LibvirtException { IntByReference nParams = new IntByReference(); -String returnValue = libvirt.virDomainGetSchedulerType(VDP, nParams); +Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams); processError(); String[] array = new String[1]; -array[0] = returnValue; +array[0] = pScheduler.getString(0); +libvirt.virFree(new PointerByReference(pScheduler)); return array; } diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index dc3a54b..e68d9ed 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -7,6 +7,7 @@ import com.sun.jna.NativeLong; import com.sun.jna.Pointer; import com.sun.jna.ptr.IntByReference; import com.sun.jna.ptr.LongByReference; +import com.sun.jna.ptr.PointerByReference; /** * The libvirt interface which is exposed via JNA. The complete API is @@ -55,6 +56,9 @@ import com.sun.jna.ptr.LongByReference; * */ public interface Libvirt extends Library { +// Memory management +void virFree(PointerByReference ptr); + // Callbacks /** * Callback interface for authorization @@ -186,7 +190,7 @@ public interface Libvirt extends Library { public String virDomainGetOSType(DomainPointer virDomainPtr); public int virDomainGetSchedulerParameters(DomainPointer virDomainPtr, virSchedParameter[] params, IntByReference nparams); -public String virDomainGetSchedulerType(DomainPointer virDomainPtr, IntByReference nparams); +Pointer virDomainGetSchedulerType(DomainPointer virDomainPtr, IntByReference nparams); public int virDomainGetUUID(DomainPointer virDomainPtr, byte[] uuidString); public int virDomainGetUUIDString(DomainPointer virDomainPtr, byte[] uuidString); public int virDomainGetVcpus(DomainPointer virDomainPtr, virVcpuInfo[] info, int maxInfo, byte[] cpumaps, int maplen); Okay, that makes sense. First do you have a small pointer
[libvirt] [PATCH 4/4] Ignore error from query-cpu-definitions
From: Daniel P. Berrange berra...@redhat.com Some architectures provide the query-cpu-definitions command, but are set to always return a GenericError from it :-( Catch this treat it as if there was an empty list of CPUs returned Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_monitor.c | 1 - src/qemu/qemu_monitor_json.c | 12 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 682d3e6..2ae6fec 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps) VIR_DEBUG(Got version %d.%d.%d (%s), major, minor, micro, NULLSTR(package)); -if (!(major = 1 minor = 1 micro = 90)) { +if (!(major = 1 || (major == 1 minor = 1))) { VIR_DEBUG(Not new enough for QMP capabilities detection); ret = 0; goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6c3f09e..85b0bc2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -843,7 +843,6 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon-watch = 0; } VIR_FORCE_CLOSE(mon-fd); -fprintf(stderr, Closing monitr\n); } /* In case another thread is waiting for its monitor command to be diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 166c431..bd52ce4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3957,6 +3957,18 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, ret = qemuMonitorJSONCommand(mon, cmd, reply); +if (ret == 0) { +/* Urgh, some QEMU architectures have the query-cpu-definitions + * command, but return 'GenericError' with string Not supported, + * instead of simply omitting the command entirely :-( + */ +if (qemuMonitorJSONHasError(reply, GenericError)) { +ret = 0; +goto cleanup; +} +ret = qemuMonitorJSONCheckError(cmd, reply); +} + if (ret == 0) ret = qemuMonitorJSONCheckError(cmd, reply); -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Remove need to pass in a virDomainObjPtr instance to qemuMonitorOpen
From: Daniel P. Berrange berra...@redhat.com The qemuMonitorOpen method only needs a virDomainObjPtr in order to access the QEMU pid. This is not critical when detecting the QEMU capabilties, so can easily be skipped Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_monitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index cd41dd7..cb121e8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -283,7 +283,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid) break; if ((errno == ENOENT || errno == ECONNREFUSED) -virProcessKill(cpid, 0) == 0) { +cpid virProcessKill(cpid, 0) == 0) { /* ENOENT : Socket may not have shown up yet * ECONNREFUSED : Leftover socket hasn't been removed yet */ continue; @@ -788,7 +788,7 @@ qemuMonitorOpen(virDomainObjPtr vm, switch (config-type) { case VIR_DOMAIN_CHR_TYPE_UNIX: hasSendFD = true; -if ((fd = qemuMonitorOpenUnix(config-data.nix.path, vm-pid)) 0) +if ((fd = qemuMonitorOpenUnix(config-data.nix.path, vm ? vm-pid : 0)) 0) return NULL; break; -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Switch to using QMP for capabilities detection
This is a fixup of the last patch in my previous series based on the feedback obtained, and some flaws I identified myself -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] Add support for detecting capablities using QMP commands
From: Daniel P. Berrange berra...@redhat.com Start a QEMU process using $QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 394 +++ 1 file changed, 358 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f0442ee..682d3e6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -35,6 +35,7 @@ #include command.h #include bitmap.h #include virnodesuspend.h +#include qemu_monitor.h #include sys/stat.h #include unistd.h @@ -189,6 +190,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, struct _qemuCaps { virObject object; +bool usedQMP; + char *binary; time_t mtime; @@ -1286,6 +1289,7 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = { }; static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = { +{ rombar, QEMU_CAPS_PCI_ROMBAR }, { configfd, QEMU_CAPS_PCI_CONFIGFD }, { bootindex, QEMU_CAPS_PCI_BOOTINDEX }, }; @@ -1866,6 +1870,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps, qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC); else if (STREQ(name, dump-guest-memory)) qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY); +else if (STREQ(name, query-spice)) +qemuCapsSet(caps, QEMU_CAPS_SPICE); +else if (STREQ(name, query-kvm)) +qemuCapsSet(caps, QEMU_CAPS_KVM); VIR_FREE(name); } VIR_FREE(commands); @@ -1898,11 +1906,117 @@ qemuCapsProbeQMPEvents(qemuCapsPtr caps, } +static int +qemuCapsProbeQMPObjects(qemuCapsPtr caps, +qemuMonitorPtr mon) +{ +int nvalues; +char **values; +size_t i; + +if ((nvalues = qemuMonitorGetObjectTypes(mon, values)) 0) +return -1; +qemuCapsProcessStringFlags(caps, + ARRAY_CARDINALITY(qemuCapsObjectTypes), + qemuCapsObjectTypes, + nvalues, values); +qemuCapsFreeStringList(nvalues, values); + +for (i = 0 ; i ARRAY_CARDINALITY(qemuCapsObjectProps); i++) { +const char *type = qemuCapsObjectProps[i].type; +if ((nvalues = qemuMonitorGetObjectProps(mon, + type, + values)) 0) +return -1; +qemuCapsProcessStringFlags(caps, + qemuCapsObjectProps[i].nprops, + qemuCapsObjectProps[i].props, + nvalues, values); +qemuCapsFreeStringList(nvalues, values); +} + +/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */ +if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV_SPICEVMC)) +qemuCapsClear(caps, QEMU_CAPS_DEVICE_SPICEVMC); + +return 0; +} + + +static int +qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ +qemuMonitorMachineInfoPtr *machines = NULL; +int nmachines = 0; +int ret = -1; +size_t i; + +if ((nmachines = qemuMonitorGetMachines(mon, machines)) 0) +goto cleanup; + +if (VIR_ALLOC_N(caps-machineTypes, nmachines) 0) { +virReportOOMError(); +goto cleanup; +} +if (VIR_ALLOC_N(caps-machineAliases, nmachines) 0) { +virReportOOMError(); +goto cleanup; +} + +for (i = 0 ; i nmachines ; i++) { +if (machines[i]-alias) { +if (!(caps-machineAliases[i] = strdup(machines[i]-name))) { +virReportOOMError(); +goto cleanup; +} +if (!(caps-machineTypes[i] = strdup(machines[i]-alias))) { +virReportOOMError(); +goto cleanup; +} +} else { +if (!(caps-machineTypes[i] = strdup(machines[i]-name))) { +virReportOOMError(); +goto cleanup; +} +} +} + +ret = 0; + +cleanup: +for (i = 0 ; i nmachines ; i++) +qemuMonitorMachineInfoFree(machines[i]); +VIR_FREE(machines); +return ret; +} + + +static int +qemuCapsProbeQMPCPUDefinitions(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ +int ncpuDefinitions; +char **cpuDefinitions; + +if ((ncpuDefinitions = qemuMonitorGetCPUDefinitions(mon, cpuDefinitions)) 0) +return -1; + +caps-ncpuDefinitions = ncpuDefinitions; +caps-cpuDefinitions = cpuDefinitions; + +return 0; +} + + int qemuCapsProbeQMP(qemuCapsPtr caps,
[libvirt] [PATCH 2/4] Avoid bogus I/O event errors when closing the QEMU monitor
From: Daniel P. Berrange berra...@redhat.com After calling qemuMonitorClose(), it is still possible for the QEMU monitor I/O event callback to get invoked. This will trigger an error message because mon-fd has been set to -1 at this point. Silently ignore the case where mon-fd is -1, likewise for mon-watch being zero. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_monitor.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index cb121e8..6c3f09e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -538,6 +538,9 @@ static void qemuMonitorUpdateWatch(qemuMonitorPtr mon) VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR; +if (!mon-watch) +return; + if (mon-lastError.code == VIR_ERR_OK) { events |= VIR_EVENT_HANDLE_READABLE; @@ -563,6 +566,11 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { #if DEBUG_IO VIR_DEBUG(Monitor %p I/O on watch %d fd %d events %d, mon, watch, fd, events); #endif +if (mon-fd == -1 || mon-watch == 0) { +qemuMonitorUnlock(mon); +virObjectUnref(mon); +return; +} if (mon-fd != fd || mon-watch != watch) { if (events (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) @@ -830,9 +838,12 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon=%p refs=%d, mon, mon-object.refs); if (mon-fd = 0) { -if (mon-watch) +if (mon-watch) { virEventRemoveHandle(mon-watch); +mon-watch = 0; +} VIR_FORCE_CLOSE(mon-fd); +fprintf(stderr, Closing monitr\n); } /* In case another thread is waiting for its monitor command to be -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] Remove need to pass in a virDomainObjPtr instance to qemuMonitorOpen
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The qemuMonitorOpen method only needs a virDomainObjPtr in order to access the QEMU pid. This is not critical when detecting the QEMU capabilties, so can easily be skipped Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_monitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 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 2/4] Avoid bogus I/O event errors when closing the QEMU monitor
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com After calling qemuMonitorClose(), it is still possible for the QEMU monitor I/O event callback to get invoked. This will trigger an error message because mon-fd has been set to -1 at this point. Silently ignore the case where mon-fd is -1, likewise for mon-watch being zero. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_monitor.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) @@ -830,9 +838,12 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon=%p refs=%d, mon, mon-object.refs); if (mon-fd = 0) { -if (mon-watch) +if (mon-watch) { virEventRemoveHandle(mon-watch); +mon-watch = 0; +} VIR_FORCE_CLOSE(mon-fd); +fprintf(stderr, Closing monitr\n); Oops. ACK with the debugging cruft elided. -- 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 3/4] Add support for detecting capablities using QMP commands
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Start a QEMU process using $QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 394 +++ 1 file changed, 358 insertions(+), 36 deletions(-) + +/* Capabilities that we assume are always enabled + * for QEMU = 1.2.0 + */ +static void +qemuCapsInitQMPBasic(qemuCapsPtr caps) +{ +qemuCapsSet(caps, QEMU_CAPS_VNC_COLON); +qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT); I think qemuCapsSetList() would be nicer than setting one bit at a time, but that doesn't change semantics. + +static int +qemuCapsInitQMP(qemuCapsPtr caps) +{ +int ret = -1; +virCommandPtr cmd = NULL; +qemuMonitorPtr mon = NULL; +int major, minor, micro; +char *package; +int status = 0; +virDomainChrSourceDef config; + +memset(config, 0, sizeof(config)); +config.type = VIR_DOMAIN_CHR_TYPE_UNIX; +config.data.nix.path = (char*)/tmp/qemu.sock; This does not seem like a safe file name (it is easily guessable). Should we instead be generating something under /var/run? +config.data.nix.listen = false; + +VIR_DEBUG(Try to get caps via QMP caps=%p, caps); + +cmd = virCommandNewArgList(caps-binary, + -S, + -no-user-config, + -nodefaults, + -nographic, + -M, none, + -qmp, unix:/tmp/qemu.sock,server,nowait, And here, we should match whatever filename we actually use. Other than that, this looks nice. -- 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 4/4] Ignore error from query-cpu-definitions
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Some architectures provide the query-cpu-definitions command, but are set to always return a GenericError from it :-( Catch this treat it as if there was an empty list of CPUs returned Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_monitor.c | 1 - src/qemu/qemu_monitor_json.c | 12 3 files changed, 13 insertions(+), 2 deletions(-) ACK. +++ b/src/qemu/qemu_capabilities.c @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps) VIR_DEBUG(Got version %d.%d.%d (%s), major, minor, micro, NULLSTR(package)); -if (!(major = 1 minor = 1 micro = 90)) { +if (!(major = 1 || (major == 1 minor = 1))) { VIR_DEBUG(Not new enough for QMP capabilities detection); ret = 0; goto cleanup; This needs to be squashed into 3/4. +++ b/src/qemu/qemu_monitor.c @@ -843,7 +843,6 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon-watch = 0; } VIR_FORCE_CLOSE(mon-fd); -fprintf(stderr, Closing monitr\n); } This needs to be squashed into 2/4. -- 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 4/4] Ignore error from query-cpu-definitions
On Fri, Sep 28, 2012 at 03:58:23PM +0100, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Some architectures provide the query-cpu-definitions command, but are set to always return a GenericError from it :-( Catch this treat it as if there was an empty list of CPUs returned Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_monitor.c | 1 - src/qemu/qemu_monitor_json.c | 12 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 682d3e6..2ae6fec 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps) VIR_DEBUG(Got version %d.%d.%d (%s), major, minor, micro, NULLSTR(package)); -if (!(major = 1 minor = 1 micro = 90)) { +if (!(major = 1 || (major == 1 minor = 1))) { VIR_DEBUG(Not new enough for QMP capabilities detection); ret = 0; goto cleanup; Opps, this was intended for the previous patch. 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 3/4] Add support for detecting capablities using QMP commands
On Fri, Sep 28, 2012 at 09:20:01AM -0600, Eric Blake wrote: On 09/28/2012 08:58 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Start a QEMU process using $QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 394 +++ 1 file changed, 358 insertions(+), 36 deletions(-) + +/* Capabilities that we assume are always enabled + * for QEMU = 1.2.0 + */ +static void +qemuCapsInitQMPBasic(qemuCapsPtr caps) +{ +qemuCapsSet(caps, QEMU_CAPS_VNC_COLON); +qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT); I think qemuCapsSetList() would be nicer than setting one bit at a time, but that doesn't change semantics. + +static int +qemuCapsInitQMP(qemuCapsPtr caps) +{ +int ret = -1; +virCommandPtr cmd = NULL; +qemuMonitorPtr mon = NULL; +int major, minor, micro; +char *package; +int status = 0; +virDomainChrSourceDef config; + +memset(config, 0, sizeof(config)); +config.type = VIR_DOMAIN_CHR_TYPE_UNIX; +config.data.nix.path = (char*)/tmp/qemu.sock; This does not seem like a safe file name (it is easily guessable). Should we instead be generating something under /var/run? Of course it isn't :-( I blame the end of week exhaustion. Locally I have an addition to make it use driver-libDir, as we do for normal monitor sockets for actual VMs. 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] node_memory: Add new parameter field to tune the new sysfs knob
Upstream kernel introduced new sysfs knob merge_across_nodes to specify if pages from different numa nodes can be merged. When set to 0, only pages which physically reside in the memory area of same NUMA node can be merged. When set to 1, pages from all nodes can be merged. This patch supports the tuning by adding new param field shm-merge-across-nodes. --- Perhaps two bool options like --disable-shm-merge-across-nodes, and --enable-shm-merge-across-nodes are good, but could the kernel sysfs knob have other values beyond 0 and 1 in future? --- include/libvirt/libvirt.h.in | 10 ++ src/nodeinfo.c | 26 +++--- tools/virsh-host.c | 20 tools/virsh.pod |5 - 4 files changed, 57 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 81f12a4..56ee2f5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4489,6 +4489,16 @@ typedef virMemoryParameter *virMemoryParameterPtr; */ # define VIR_NODE_MEMORY_SHARED_FULL_SCANS shm_full_scans +/* VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES: + * + * Macro for typed parameter that represents whether pages from + * different NUMA nodes can be merged, when its value is 0, only + * pages which physically reside in the memory area of same NUMA + * node are merged, otherwise pages from all nodes can be merged. + */ +# define VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES shm_merge_across_nodes + + int virNodeGetMemoryParameters(virConnectPtr conn, virTypedParameterPtr params, int *nparams, diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 021eb05..6002789 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1004,6 +1004,13 @@ nodeSetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED, /* Out of memory */ if (ret == -2) return -1; +} else if (STREQ(param-field, + VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES)) { +ret = nodeSetMemoryParameterValue(merge_across_nodes, param); + +/* Out of memory */ +if (ret == -2) +return -1; } } @@ -1039,8 +1046,9 @@ nodeGetMemoryParameterValue(const char *field, if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; -if (STREQ(field, pages_to_scan) || -STREQ(field, sleep_millisecs)) +if (STREQ(field, pages_to_scan) || +STREQ(field, sleep_millisecs) || +STREQ(field, merge_across_nodes)) rc = virStrToLong_ui(buf, NULL, 10, (unsigned int *)value); else if (STREQ(field, pages_shared)|| STREQ(field, pages_sharing) || @@ -1063,7 +1071,7 @@ cleanup: } #endif -#define NODE_MEMORY_PARAMETERS_NUM 7 +#define NODE_MEMORY_PARAMETERS_NUM 8 int nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED, virTypedParameterPtr params ATTRIBUTE_UNUSED, @@ -1075,6 +1083,7 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED, #ifdef __linux__ unsigned int pages_to_scan; unsigned int sleep_millisecs; +unsigned int merge_across_nodes; unsigned long long pages_shared; unsigned long long pages_sharing; unsigned long long pages_unshared; @@ -1168,6 +1177,17 @@ nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED, break; +case 7: +if (nodeGetMemoryParameterValue(merge_across_nodes, +merge_across_nodes) 0) +return -1; + +if (virTypedParameterAssign(param, VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES, +VIR_TYPED_PARAM_UINT, merge_across_nodes) 0) +return -1; + +break; + default: break; } diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 2c46336..62ecafc 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -900,6 +900,8 @@ static const vshCmdOptDef opts_node_memory_tune[] = { {shm-sleep-millisecs, VSH_OT_INT, VSH_OFLAG_NONE, N_(number of millisecs the shared memory service should sleep before next scan)}, +{shm-merge-across-nodes, VSH_OT_INT, VSH_OFLAG_NONE, + N_(Specifies if pages from different numa nodes can be merged)}, {NULL, 0, 0, NULL} }; @@ -911,6 +913,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; unsigned int shm_pages_to_scan = 0; unsigned int shm_sleep_millisecs = 0; +unsigned int shm_merge_across_nodes = 0; bool ret = false; int i = 0; @@ -926,12 +929,21 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) return false; } +if (vshCommandOptUInt(cmd, shm-merge-across-nodes, + shm_merge_across_nodes) 0) { +vshError(ctl, %s, _(invalid
Re: [libvirt] virsh --connect not working
On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.comwrote: On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote: I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Try collecting some debugging output using LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system It should give a better idea why it failed. Not getting any useful info: $ ps awwux | grep libvirtd root 3396 0.0 0.4 103100 32248 - I 8:24AM 0:01.05 libvirtd -v -d $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor Tried 1-4 debug levels with the same response. Thanks, Hiren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add support for detecting capablities using QMP commands
From: Daniel P. Berrange berra...@redhat.com Start a QEMU process using $QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 419 +++ src/qemu/qemu_capabilities.h | 5 +- src/qemu/qemu_driver.c | 2 +- 3 files changed, 385 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f0442ee..c977e25 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -35,6 +35,7 @@ #include command.h #include bitmap.h #include virnodesuspend.h +#include qemu_monitor.h #include sys/stat.h #include unistd.h @@ -189,6 +190,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, struct _qemuCaps { virObject object; +bool usedQMP; + char *binary; time_t mtime; @@ -210,6 +213,7 @@ struct _qemuCaps { struct _qemuCapsCache { virMutex lock; virHashTablePtr binaries; +char *libDir; }; @@ -1286,6 +1290,7 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = { }; static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = { +{ rombar, QEMU_CAPS_PCI_ROMBAR }, { configfd, QEMU_CAPS_PCI_CONFIGFD }, { bootindex, QEMU_CAPS_PCI_BOOTINDEX }, }; @@ -1866,6 +1871,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps, qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC); else if (STREQ(name, dump-guest-memory)) qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY); +else if (STREQ(name, query-spice)) +qemuCapsSet(caps, QEMU_CAPS_SPICE); +else if (STREQ(name, query-kvm)) +qemuCapsSet(caps, QEMU_CAPS_KVM); VIR_FREE(name); } VIR_FREE(commands); @@ -1898,11 +1907,117 @@ qemuCapsProbeQMPEvents(qemuCapsPtr caps, } +static int +qemuCapsProbeQMPObjects(qemuCapsPtr caps, +qemuMonitorPtr mon) +{ +int nvalues; +char **values; +size_t i; + +if ((nvalues = qemuMonitorGetObjectTypes(mon, values)) 0) +return -1; +qemuCapsProcessStringFlags(caps, + ARRAY_CARDINALITY(qemuCapsObjectTypes), + qemuCapsObjectTypes, + nvalues, values); +qemuCapsFreeStringList(nvalues, values); + +for (i = 0 ; i ARRAY_CARDINALITY(qemuCapsObjectProps); i++) { +const char *type = qemuCapsObjectProps[i].type; +if ((nvalues = qemuMonitorGetObjectProps(mon, + type, + values)) 0) +return -1; +qemuCapsProcessStringFlags(caps, + qemuCapsObjectProps[i].nprops, + qemuCapsObjectProps[i].props, + nvalues, values); +qemuCapsFreeStringList(nvalues, values); +} + +/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */ +if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV_SPICEVMC)) +qemuCapsClear(caps, QEMU_CAPS_DEVICE_SPICEVMC); + +return 0; +} + + +static int +qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ +qemuMonitorMachineInfoPtr *machines = NULL; +int nmachines = 0; +int ret = -1; +size_t i; + +if ((nmachines = qemuMonitorGetMachines(mon, machines)) 0) +goto cleanup; + +if (VIR_ALLOC_N(caps-machineTypes, nmachines) 0) { +virReportOOMError(); +goto cleanup; +} +if (VIR_ALLOC_N(caps-machineAliases, nmachines) 0) { +virReportOOMError(); +goto cleanup; +} + +for (i = 0 ; i nmachines ; i++) { +if (machines[i]-alias) { +if (!(caps-machineAliases[i] = strdup(machines[i]-name))) { +virReportOOMError(); +goto cleanup; +} +if (!(caps-machineTypes[i] = strdup(machines[i]-alias))) { +virReportOOMError(); +goto cleanup; +} +} else { +if (!(caps-machineTypes[i] = strdup(machines[i]-name))) { +virReportOOMError(); +goto cleanup; +} +} +} + +ret = 0; + +cleanup: +for (i = 0 ; i nmachines ; i++) +qemuMonitorMachineInfoFree(machines[i]); +VIR_FREE(machines); +return ret; +} + + +static int +qemuCapsProbeQMPCPUDefinitions(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ +int ncpuDefinitions; +char **cpuDefinitions; + +if ((ncpuDefinitions =
Re: [libvirt] virsh --connect not working
On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote: On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.comwrote: On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote: I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Try collecting some debugging output using LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system It should give a better idea why it failed. Not getting any useful info: $ ps awwux | grep libvirtd root 3396 0.0 0.4 103100 32248 - I 8:24AM 0:01.05 libvirtd -v -d $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor Tried 1-4 debug levels with the same response. Sounds like sudo is stripping the env variable. Try running it as root directly, without sudo. 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] Add support for detecting capablities using QMP commands
On 09/28/2012 09:43 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Start a QEMU process using $QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 419 +++ src/qemu/qemu_capabilities.h | 5 +- src/qemu/qemu_driver.c | 2 +- 3 files changed, 385 insertions(+), 41 deletions(-) +char *monarg = NULL; +char *monpath = NULL; + +if (virAsprintf(monpath, %s/%s, libDir, capabilities.monitor.sock) 0) { +virReportOOMError(); +goto cleanup; +} +if (virAsprintf(monarg, unix:%s,server,nowait, monpath) 0) { +virReportOOMError(); +goto cleanup; +} I would have used virCommandAddArgFormat() instead of creating my own temporary string here, but that's minor. ACK; you fixed my concerns from the previous version. Also, be sure you run 'make check'; somewhere between commit de29867 and 7022b09, you broke qemuhelptest (I'm assuming it will get fixed soon, perhaps by this commit, so I'm not chasing which commit did the actual breakage): 16) QEMU Help String Parsing qemu-1.2.0 ... qemu-1.2.0: computed flags do not match: got 0xb0f2f1bffefff4cffd62fffddc6e, expected 0xb0f2f1bffefff4effd72fffddc6e Missing flag 36 Missing flag 53 FAILED -- 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] virsh --connect not working
On Fri, Sep 28, 2012 at 8:40 AM, Daniel P. Berrange berra...@redhat.comwrote: On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote: On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote: I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Try collecting some debugging output using LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system It should give a better idea why it failed. Not getting any useful info: $ ps awwux | grep libvirtd root 3396 0.0 0.4 103100 32248 - I 8:24AM 0:01.05 libvirtd -v -d $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor Tried 1-4 debug levels with the same response. Sounds like sudo is stripping the env variable. Try running it as root directly, without sudo. Aah, right, o/p is huge so putting it in pastebin: http://pastebin.com/YVrK0fRb Thanks a lot, Hiren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh --connect not working
On Fri, Sep 28, 2012 at 08:51:40AM -0700, hiren panchasara wrote: On Fri, Sep 28, 2012 at 8:40 AM, Daniel P. Berrange berra...@redhat.comwrote: On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote: On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote: I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Try collecting some debugging output using LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system It should give a better idea why it failed. Not getting any useful info: $ ps awwux | grep libvirtd root 3396 0.0 0.4 103100 32248 - I 8:24AM 0:01.05 libvirtd -v -d $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor Tried 1-4 debug levels with the same response. Sounds like sudo is stripping the env variable. Try running it as root directly, without sudo. Aah, right, o/p is huge so putting it in pastebin: http://pastebin.com/YVrK0fRb Looks somewhat like your libvirt has been built without support for the QEMU driver 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] Fix QEMU test with 1.2.0 help output
From: Daniel P. Berrange berra...@redhat.com The help output for QEMU 1.2.0 changed 'pci-assign' to 'kvm-pci-assign'. Since the new capabilities code does exact device name matching instead of substring matching, this caused the capabilities to go missing. Pushed as a build break fix Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_capabilities.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f0442ee..cac4917 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1321,6 +1321,8 @@ static struct qemuCapsObjectTypeProps qemuCapsObjectProps[] = { ARRAY_CARDINALITY(qemuCapsObjectPropsVirtioNet) }, { pci-assign, qemuCapsObjectPropsPciAssign, ARRAY_CARDINALITY(qemuCapsObjectPropsPciAssign) }, +{ kvm-pci-assign, qemuCapsObjectPropsPciAssign, + ARRAY_CARDINALITY(qemuCapsObjectPropsPciAssign) }, { scsi-disk, qemuCapsObjectPropsScsiDisk, ARRAY_CARDINALITY(qemuCapsObjectPropsScsiDisk) }, { ide-drive, qemuCapsObjectPropsIDEDrive, -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] openvswitch: Add utility functions for getting and setting Open vSwitch per-port data
On 09/21/2012 05:16 PM, Kyle Mestery wrote: Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Signed-off-by: Kyle Mestery kmest...@cisco.com --- src/libvirt_private.syms| 2 ++ src/util/virnetdevopenvswitch.c | 70 + src/util/virnetdevopenvswitch.h | 6 3 files changed, 78 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0b6068d..54c591b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1479,7 +1479,9 @@ virNetDevMacVLanVPortProfileRegisterCallback; # virnetdevopenvswitch.h virNetDevOpenvswitchAddPort; +virNetDevOpenvswitchGetMigrateData; virNetDevOpenvswitchRemovePort; +virNetDevOpenvswitchSetMigrateData; # virnetdevtap.h diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index a6993b6..c30cbaa 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -179,3 +179,73 @@ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const ch virCommandFree(cmd); return ret; } + +/** + * virNetDevOpenvswitchGetMigrateData: + * @migrate: a pointer to store the data into, allocated by this function + * @ifname: name of the interface for which data is being migrated + * + * Allocates data to be migrated specific to Open vSwitch + * + * Returns 0 in case of success or -1 in case of failure + */ +int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) +{ +virCommandPtr cmd = NULL; +int ret = 0; + +cmd = virCommandNewArgList(OVSVSCTL, get, Interface, + ifname, external_ids:PortData, NULL); + +virCommandSetOutputBuffer(cmd, migrate); + +/* Run the command */ +if (virCommandRun(cmd, NULL) 0) { +virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _(Unable to run command to get OVS port data for + interface %s), ifname); +ret = -1; +goto error; +} + +/* Wipeout the newline */ +(*migrate)[strlen(*migrate) - 1] = '\0'; + +error: +return ret; +} + +/** + * virNetDevOpenvswitchSetMigrateData: + * @migrate: the data which was transferred during migration + * @ifname: the name of the interface the data is associated with + * + * Repopulates OVS per-port data on destination host + * + * Returns 0 in case of success or -1 in case of failure + */ +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) +{ +virCommandPtr cmd = NULL; +int ret = 0; +virBufferPtr buf; + +if (VIR_ALLOC(buf) 0) { +ret = -1; +goto error; +} + +virBufferAsprintf(buf, external_ids:PortData=%s, migrate); + +cmd = virCommandNewArgList(OVSVSCTL, set, Interface, ifname, + virBufferCurrentContent(buf), NULL); You need to add the --timeout=5 option as I did for add-port and del-port, to avoid an infinite wait when ovs-vswitchd isn't running. ACK with that change. +/* Run the command */ +if ((ret = virCommandRun(cmd, NULL)) 0) { +virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _(Unable to run command to set OVS port data for + interface %s), ifname); +} + +error: +return ret; +} diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 7e5b618..147cd6f 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -42,4 +42,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, int virNetDevOpenvswitchRemovePort(const char *brname, const char *ifname) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + #endif /* __VIR_NETDEV_OPENVSWITCH_H__ */ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu_migration: Add hooks to transport network port data during migration
On 09/21/2012 05:16 PM, Kyle Mestery wrote: Add the ability for the Qemu V3 migration protocol to include transporting network configuration. A generic framework is proposed with this patch to allow for the transfer of opaque data. Functionally this all looks good (and sounds like it lives up to that in practice :-). There are some code style issues, though... Signed-off-by: Kyle Mestery kmest...@cisco.com --- src/qemu/qemu_migration.c | 278 +- 1 file changed, 276 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8e85875..06981db 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -70,6 +70,7 @@ enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS, QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE, QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT, +QEMU_MIGRATION_COOKIE_FLAG_NETWORK, QEMU_MIGRATION_COOKIE_FLAG_LAST }; @@ -77,12 +78,13 @@ enum qemuMigrationCookieFlags { VIR_ENUM_DECL(qemuMigrationCookieFlag); VIR_ENUM_IMPL(qemuMigrationCookieFlag, QEMU_MIGRATION_COOKIE_FLAG_LAST, - graphics, lockstate, persistent); + graphics, lockstate, persistent, network); enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_GRAPHICS = (1 QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS), QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE), QEMU_MIGRATION_COOKIE_PERSISTENT = (1 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT), +QEMU_MIGRATION_COOKIE_NETWORK = (1 QEMU_MIGRATION_COOKIE_FLAG_NETWORK), }; typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; @@ -95,6 +97,35 @@ struct _qemuMigrationCookieGraphics { char *tlsSubject; }; +VIR_ENUM_DECL(virInterface); +VIR_ENUM_IMPL(virInterface, VIR_NETDEV_VPORT_PROFILE_LAST, + none, + 802.1QBg, + 802.1QBh, + openvswitch); (I'd never before considered using VIR_ENUM_*() and giving an enum type identifier that didn't match the enum being translated) Since you're using the enum virNetDevVPortProfile for these values anyway, you should just use the already-existing functions: virNetDevVPortTypeFromString virNetDevVPortTypeToString (these are declared/defined in virnetdevvportprofile.h, and included in libvirt_private.syms). + +typedef struct _qemuMigrationCookieNetdata qemuMigrationCookieNetdata; +typedef qemuMigrationCookieNetdata *qemuMigrationCookieNetdataPtr; +struct _qemuMigrationCookieNetdata { +/* What type is each interace ? virInterface above ... */ +char *interfacetype; 1) Since the xml attribute is called vporttype, you should give the variable in the struct the same name, to avoid confusion. 2) Internally, we usually store the enum value rather than the string, and translate to/from string form during XML parse/format. (For some reason, normal usage is to specify it in the struct as an int, with a comment giving the actual type: int vporttype; /* enum virNetDevVPortProfile */ I'm not sure why that is, to be truthful - Eric or Dan?) + +/* + * Array of pointers to saved data. Each VIF will have it's own + * data to transfer. + */ +char *portdata; +}; + +typedef struct _qemuMigrationCookieNetwork qemuMigrationCookieNetwork; +typedef qemuMigrationCookieNetwork *qemuMigrationCookieNetworkPtr; +struct _qemuMigrationCookieNetwork { +/* How many virtual NICs are we saving data for? */ +int nnets; + +qemuMigrationCookieNetdataPtr *net; +}; + typedef struct _qemuMigrationCookie qemuMigrationCookie; typedef qemuMigrationCookie *qemuMigrationCookiePtr; struct _qemuMigrationCookie { @@ -120,6 +151,9 @@ struct _qemuMigrationCookie { /* If (flags QEMU_MIGRATION_COOKIE_PERSISTENT) */ virDomainDefPtr persistent; + +/* If (flags QEMU_MIGRATION_COOKIE_NETWORK) */ +qemuMigrationCookieNetworkPtr network; }; static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) @@ -132,6 +166,28 @@ static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) } +static void qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr + network) +{ +int i; + +if (!network) +return; + +for (i = 0; i network-nnets; i++) { +if (network-net[i]) { +if (network-net[i]-interfacetype) +VIR_FREE(network-net[i]-interfacetype); VIR_FREE() automatically checks for NULL, and is a NOP if the pointer is NULL (it also sets the pointer to NULL when it's finished, so calling it twice doesn't create a problem. make syntax-check will actually find such redundant null-check-before-free and complain about it. So, you should remove all checks for NULL before calling
Re: [libvirt] [PATCH 3/3] qemu_migration: Transport Open vSwitch per-port data during live migration
On 09/21/2012 05:16 PM, Kyle Mestery wrote: Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData(). I like how the first part is all re-usable infrastructure, and the final patch that actually makes things work is so small! That implies that it will be easy to add other new functionality in the future... Signed-off-by: Kyle Mestery kmest...@cisco.com --- src/qemu/qemu_migration.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 06981db..33cdb20 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -322,7 +322,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, { qemuMigrationCookieNetworkPtr mig; int i; -virDomainNetDefPtr netptr ATTRIBUTE_UNUSED; +virDomainNetDefPtr netptr; const char *interfacetype; if (VIR_ALLOC(mig) 0) @@ -357,7 +357,13 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, mig-net[i]-portdata = NULL; break; case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: -mig-net[i]-portdata = NULL; +if (virNetDevOpenvswitchGetMigrateData(mig-net[i]-portdata, + netptr-ifname) != 0) { +virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _(Unable to run command to get OVS port data for + interface %s), netptr-ifname); +goto error; +} break; default: mig-net[i]-portdata = NULL; @@ -366,6 +372,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, } } +error: return mig; no_memory: @@ -1284,7 +1291,7 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED, virDomainObjPtr vm, qemuMigrationCookiePtr cookie) { -virDomainNetDefPtr netptr ATTRIBUTE_UNUSED; +virDomainNetDefPtr netptr; int ret = 0; These days we tend to 1) initialize ret = -1 (assume failure), call the early exit label cleanup rather than error (if a successful exit will run through that label - it avoids confusion when someone reads the code later), and put a ret = 0; on the line just before cleanup:. That way, you don't need to put in all the ret = -1; lines whenever there is a failure. ACK with that change (and after re-basing to account for the changes requested in 1/3). int i; int interfacetype; @@ -1305,7 +1312,14 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED, cookie-network-net[i]-portdata = NULL; break; case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: -cookie-network-net[i]-portdata = NULL; +if (virNetDevOpenvswitchSetMigrateData(cookie-network-net[i]-portdata, + netptr-ifname) != 0) { +virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _(Unable to run command to set OVS port data for + interface %s), netptr-ifname); +ret = -1; +goto error; +} break; default: cookie-network-net[i]-portdata = NULL; @@ -1314,6 +1328,7 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED, } } +error: return ret; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh --connect not working
On Fri, Sep 28, 2012 at 8:53 AM, Daniel P. Berrange berra...@redhat.comwrote: On Fri, Sep 28, 2012 at 08:51:40AM -0700, hiren panchasara wrote: On Fri, Sep 28, 2012 at 8:40 AM, Daniel P. Berrange berra...@redhat.com wrote: On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote: On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote: I've installed libvirt-0.9.13 from ports on my freebsd machine. I started libvirtd: $ ps awwux | grep libvirtd root 11470 0.0 0.4 103100 31948 - I10:41PM 0:00.35 libvirtd -v -d $ sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor I am sure I am missing something obvious. Try collecting some debugging output using LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system It should give a better idea why it failed. Not getting any useful info: $ ps awwux | grep libvirtd root 3396 0.0 0.4 103100 32248 - I 8:24AM 0:01.05 libvirtd -v -d $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system error: no connection driver available for No connection for URI qemu:///system error: failed to connect to the hypervisor Tried 1-4 debug levels with the same response. Sounds like sudo is stripping the env variable. Try running it as root directly, without sudo. Aah, right, o/p is huge so putting it in pastebin: http://pastebin.com/YVrK0fRb Looks somewhat like your libvirt has been built without support for the QEMU driver Daniel Libvirt is currently incomplete for FreeBSD, and there is an effort to get it fully supported right now. Libvirtd daemon should not start, and I would be surprised if it did. QEMU drivers aren't enabled either, but if you would like to contribute anything please contact me and I can get you in contact with the right people to contribute. There is also efforts to implement the BSD Hypervisor into the libvirt codebase, as well. Thanks! -jgh -- Jason Helfman | FreeBSD Committer j...@freebsd.org | http://people.freebsd.org/~jgh -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh --connect not working
On Fri, Sep 28, 2012 at 9:14 AM, Jason Helfman j...@freebsd.org wrote: On Fri, Sep 28, 2012 at 8:53 AM, Daniel P. Berrange berra...@redhat.comwrote: Looks somewhat like your libvirt has been built without support for the QEMU driver Daniel Thanks for looking into it, Daniel. Libvirt is currently incomplete for FreeBSD, and there is an effort to get it fully supported right now. Libvirtd daemon should not start, and I would be surprised if it did. QEMU drivers aren't enabled either, but if you would like to contribute anything please contact me and I can get you in contact with the right people to contribute. There is also efforts to implement the BSD Hypervisor into the libvirt codebase, as well. Hi Jason, Sending you separate email to talk about this. Thanks, Hiren -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Transport network port data during live migration
On Fri, Sep 21, 2012 at 05:16:45PM -0400, Kyle Mestery wrote: This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together. The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed. An example of the XML being transported is shown here: network interface index='1' vporttype='openvswitch' netdata='blah blah blah'/ interface index='7' vporttype='openvswitch' netdata='blah blah blah'/ /network Could you give an example of what goes in the netdata='blah blah blah' part of the XML ? I'm interesting in seeing the type of data, and its indicitive size 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 1/3] qemu_migration: Add hooks to transport network port data during migration
On 09/28/2012 10:14 AM, Laine Stump wrote: On 09/21/2012 05:16 PM, Kyle Mestery wrote: Add the ability for the Qemu V3 migration protocol to include transporting network configuration. A generic framework is proposed with this patch to allow for the transfer of opaque data. Functionally this all looks good (and sounds like it lives up to that in practice :-). There are some code style issues, though... 2) Internally, we usually store the enum value rather than the string, and translate to/from string form during XML parse/format. (For some reason, normal usage is to specify it in the struct as an int, with a comment giving the actual type: int vporttype; /* enum virNetDevVPortProfile */ I'm not sure why that is, to be truthful - Eric or Dan?) For public API - ABI stability. The C standard does not give any guarantees about the sizeof an enum member in a struct, and at least gcc allows compiler flags that change struct layout based on whether enums are packed to the smallest integer size that holds all the values of the enum or is as wide as an int. Since we don't control the compiler flags used by a client app that includes our public headers, we're better off avoiding the issue. Using 'int' instead of the enum type frees us from the ABI questions. For private API - consistent coding style. Besides, C is not as typesafe as C++ with regards to enums, so it's not like we're losing much functionality. -- 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] [libvirt-glib 1/2] gobject: Handle PMSUSPENDED state from libvirt
Emit a signal when switching to the PMSUSPENDED state, and add an enum entry to describe this state. This avoids runtime warnings with newer libvirt. --- libvirt-gobject/libvirt-gobject-connection.c | 7 +++ libvirt-gobject/libvirt-gobject-domain.c | 11 +++ libvirt-gobject/libvirt-gobject-domain.h | 19 +++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 428ae38..ad7aa07 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -389,6 +389,13 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, case VIR_DOMAIN_EVENT_SHUTDOWN: break; +case VIR_DOMAIN_EVENT_PMSUSPENDED: +if (detail == VIR_DOMAIN_EVENT_PMSUSPENDED_MEMORY) +g_signal_emit_by_name(gdom, pmsuspended::memory); +else +g_warn_if_reached(); +break; + default: g_warn_if_reached(); } diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index bcfad2a..f8ad493 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -55,6 +55,7 @@ enum { VIR_RESUMED, VIR_STOPPED, VIR_UPDATED, +VIR_PMSUSPENDED, LAST_SIGNAL }; @@ -225,6 +226,16 @@ static void gvir_domain_class_init(GVirDomainClass *klass) G_TYPE_NONE, 0); +signals[VIR_PMSUSPENDED] = g_signal_new(pmsuspended, +G_OBJECT_CLASS_TYPE(object_class), +G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE | +G_SIGNAL_NO_HOOKS | G_SIGNAL_DETAILED, +G_STRUCT_OFFSET(GVirDomainClass, pmsuspended), +NULL, NULL, +g_cclosure_marshal_VOID__VOID, +G_TYPE_NONE, +0); + g_type_class_add_private(klass, sizeof(GVirDomainPrivate)); } diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 7810d1b..fc2db7c 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -66,19 +66,22 @@ struct _GVirDomainClass void (*resumed)(GVirDomain *dom); void (*updated)(GVirDomain *dom); void (*suspended)(GVirDomain *dom); +void (*pmsuspended)(GVirDomain *dom); -gpointer padding[20]; +gpointer padding[19]; }; typedef enum { -GVIR_DOMAIN_STATE_NONE= 0, /* no state */ -GVIR_DOMAIN_STATE_RUNNING = 1, /* the domain is running */ -GVIR_DOMAIN_STATE_BLOCKED = 2, /* the domain is blocked on resource */ -GVIR_DOMAIN_STATE_PAUSED = 3, /* the domain is paused by user */ -GVIR_DOMAIN_STATE_SHUTDOWN= 4, /* the domain is being shut down */ -GVIR_DOMAIN_STATE_SHUTOFF = 5, /* the domain is shut off */ -GVIR_DOMAIN_STATE_CRASHED = 6 /* the domain is crashed */ +GVIR_DOMAIN_STATE_NONE= 0, /* no state */ +GVIR_DOMAIN_STATE_RUNNING = 1, /* the domain is running */ +GVIR_DOMAIN_STATE_BLOCKED = 2, /* the domain is blocked on resource */ +GVIR_DOMAIN_STATE_PAUSED = 3, /* the domain is paused by user */ +GVIR_DOMAIN_STATE_SHUTDOWN= 4, /* the domain is being shut down */ +GVIR_DOMAIN_STATE_SHUTOFF = 5, /* the domain is shut off */ +GVIR_DOMAIN_STATE_CRASHED = 6, /* the domain is crashed */ +GVIR_DOMAIN_STATE_PMSUSPENDED = 7 /* the domain is suspended by guest + power management */ } GVirDomainState; -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 2/2] gobject: Emit a signal on WAKEUP events
This avoids a runtime warning about this kind of event not being handled. --- libvirt-gobject/libvirt-gobject-connection.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index ad7aa07..9628989 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -327,6 +327,8 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED, g_signal_emit_by_name(gdom, started::restored); else if (detail == VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT) g_signal_emit_by_name(gdom, started::from-snapshot); +else if (detail == VIR_DOMAIN_EVENT_STARTED_WAKEUP) +g_signal_emit_by_name(gdom, started::wakeup); else g_warn_if_reached(); break; -- 1.7.11.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Patch to default selinuxfs mount point to /sys/fs/selinux
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Currently if you build on a machine that does not support SELinux we end up with the default mount point being /selinux, since this is moved to /sys/fs/selinux, we should start defaulting there. I believe this is causing a bug in libvirt-lxc when /selinux does not exists, even though /sys/fs/selinux exists. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlBl6F8ACgkQrlYvE4MpobPz0ACgxPdENx/KDtQY7YGT7BDXoLP3 AVIAoJwUvTib72U0VJ3dnVoGU0PYjMXZ =XU8L -END PGP SIGNATURE- diff --git a/.gnulib b/.gnulib index 440a1db..dbd9144 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 +Subproject commit dbd914496c99c52220e5f5ba4121d6cb55fb3beb diff --git a/configure.ac b/configure.ac index ae26de7..5cc5cbe 100644 --- a/configure.ac +++ b/configure.ac @@ -1455,11 +1455,10 @@ fi if test $with_selinux = yes; then AC_MSG_CHECKING([SELinux mount point]) if test $with_selinux_mount = check || test -z $with_selinux_mount; then -if test -d /sys/fs/selinux ; then - SELINUX_MOUNT=/sys/fs/selinux -else - SELINUX_MOUNT=/selinux -fi + SELINUX_MOUNT=/sys/fs/selinux + if ! test -d ${SELINUX_MOUNT} test -d /selinux ; then + SELINUX_MOUNT=/selinux + fi else SELINUX_MOUNT=$with_selinux_mount fi -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Patch to default selinuxfs mount point to /sys/fs/selinux
On 09/28/2012 12:11 PM, Daniel J Walsh wrote: OpenPGP: *Attachments to this message have not been signed or encrypted* Currently if you build on a machine that does not support SELinux we end up with the default mount point being /selinux, since this is moved to /sys/fs/selinux, we should start defaulting there. The spec files are already doing this correctly, so this patch will only help someone doing a self-build rather than relying on a spec file, and only then if selinux is not mounted on the machine where the build is done, in relation to where the built binary is run. +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42 +Subproject commit dbd914496c99c52220e5f5ba4121d6cb55fb3beb This should not be part of the patch. diff --git a/configure.ac b/configure.ac index ae26de7..5cc5cbe 100644 --- a/configure.ac +++ b/configure.ac @@ -1455,11 +1455,10 @@ fi if test $with_selinux = yes; then AC_MSG_CHECKING([SELinux mount point]) if test $with_selinux_mount = check || test -z $with_selinux_mount; then -if test -d /sys/fs/selinux ; then - SELINUX_MOUNT=/sys/fs/selinux -else - SELINUX_MOUNT=/selinux -fi + SELINUX_MOUNT=/sys/fs/selinux + if ! test -d ${SELINUX_MOUNT} test -d /selinux ; then + SELINUX_MOUNT=/selinux + fi else ACK to this hunk, and I will push 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
Re: [libvirt] [PATCH 0/3] Transport network port data during live migration
On Sep 28, 2012, at 11:42 AM, Daniel P. Berrange wrote: On Fri, Sep 21, 2012 at 05:16:45PM -0400, Kyle Mestery wrote: This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together. The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed. An example of the XML being transported is shown here: network interface index='1' vporttype='openvswitch' netdata='blah blah blah'/ interface index='7' vporttype='openvswitch' netdata='blah blah blah'/ /network Could you give an example of what goes in the netdata='blah blah blah' part of the XML ? I'm interesting in seeing the type of data, and its indicitive size Daniel Hi Daniel: From a hypothetical angle: As an example, an OpenFlow controller may have certain information about the port, specific to this controller, which it may want to store with the port itself on the host. This especially true if an agent exists on the host which needs to read this data, update it, and use it to perform some tasks. It's convenient to have this data stored as close to the port itself, which in this case is the OVS DB, and having it transferred as part of the migration protocol is also very handy. Thanks, Kyle -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu_migration: Add hooks to transport network port data during migration
On Sep 28, 2012, at 11:14 AM, Laine Stump wrote: On 09/21/2012 05:16 PM, Kyle Mestery wrote: Add the ability for the Qemu V3 migration protocol to include transporting network configuration. A generic framework is proposed with this patch to allow for the transfer of opaque data. Functionally this all looks good (and sounds like it lives up to that in practice :-). There are some code style issues, though… Thanks for all the comments Laine. I'll address them all and send out new versions of the patches ASAP. Kyle Signed-off-by: Kyle Mestery kmest...@cisco.com --- src/qemu/qemu_migration.c | 278 +- 1 file changed, 276 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8e85875..06981db 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -70,6 +70,7 @@ enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS, QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE, QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT, +QEMU_MIGRATION_COOKIE_FLAG_NETWORK, QEMU_MIGRATION_COOKIE_FLAG_LAST }; @@ -77,12 +78,13 @@ enum qemuMigrationCookieFlags { VIR_ENUM_DECL(qemuMigrationCookieFlag); VIR_ENUM_IMPL(qemuMigrationCookieFlag, QEMU_MIGRATION_COOKIE_FLAG_LAST, - graphics, lockstate, persistent); + graphics, lockstate, persistent, network); enum qemuMigrationCookieFeatures { QEMU_MIGRATION_COOKIE_GRAPHICS = (1 QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS), QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE), QEMU_MIGRATION_COOKIE_PERSISTENT = (1 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT), +QEMU_MIGRATION_COOKIE_NETWORK = (1 QEMU_MIGRATION_COOKIE_FLAG_NETWORK), }; typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics; @@ -95,6 +97,35 @@ struct _qemuMigrationCookieGraphics { char *tlsSubject; }; +VIR_ENUM_DECL(virInterface); +VIR_ENUM_IMPL(virInterface, VIR_NETDEV_VPORT_PROFILE_LAST, + none, + 802.1QBg, + 802.1QBh, + openvswitch); (I'd never before considered using VIR_ENUM_*() and giving an enum type identifier that didn't match the enum being translated) Since you're using the enum virNetDevVPortProfile for these values anyway, you should just use the already-existing functions: virNetDevVPortTypeFromString virNetDevVPortTypeToString (these are declared/defined in virnetdevvportprofile.h, and included in libvirt_private.syms). + +typedef struct _qemuMigrationCookieNetdata qemuMigrationCookieNetdata; +typedef qemuMigrationCookieNetdata *qemuMigrationCookieNetdataPtr; +struct _qemuMigrationCookieNetdata { +/* What type is each interace ? virInterface above ... */ +char *interfacetype; 1) Since the xml attribute is called vporttype, you should give the variable in the struct the same name, to avoid confusion. 2) Internally, we usually store the enum value rather than the string, and translate to/from string form during XML parse/format. (For some reason, normal usage is to specify it in the struct as an int, with a comment giving the actual type: int vporttype; /* enum virNetDevVPortProfile */ I'm not sure why that is, to be truthful - Eric or Dan?) + +/* + * Array of pointers to saved data. Each VIF will have it's own + * data to transfer. + */ +char *portdata; +}; + +typedef struct _qemuMigrationCookieNetwork qemuMigrationCookieNetwork; +typedef qemuMigrationCookieNetwork *qemuMigrationCookieNetworkPtr; +struct _qemuMigrationCookieNetwork { +/* How many virtual NICs are we saving data for? */ +int nnets; + +qemuMigrationCookieNetdataPtr *net; +}; + typedef struct _qemuMigrationCookie qemuMigrationCookie; typedef qemuMigrationCookie *qemuMigrationCookiePtr; struct _qemuMigrationCookie { @@ -120,6 +151,9 @@ struct _qemuMigrationCookie { /* If (flags QEMU_MIGRATION_COOKIE_PERSISTENT) */ virDomainDefPtr persistent; + +/* If (flags QEMU_MIGRATION_COOKIE_NETWORK) */ +qemuMigrationCookieNetworkPtr network; }; static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) @@ -132,6 +166,28 @@ static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) } +static void qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr + network) +{ +int i; + +if (!network) +return; + +for (i = 0; i network-nnets; i++) { +if (network-net[i]) { +if (network-net[i]-interfacetype) +VIR_FREE(network-net[i]-interfacetype); VIR_FREE() automatically checks for NULL, and is a NOP if the pointer is NULL (it also sets the pointer to NULL when it's finished, so calling it twice doesn't create a problem.
Re: [libvirt] TSC scaling interface to management
On Tue, Sep 25, 2012 at 11:08:58AM +0100, Daniel P. Berrange wrote: On Wed, Sep 12, 2012 at 12:39:39PM -0300, Marcelo Tosatti wrote: HW TSC scaling is a feature of AMD processors that allows a multiplier to be specified to the TSC frequency exposed to the guest. KVM also contains provision to trap TSC (KVM: Infrastructure for software and hardware based TSC rate scaling cc578287e3224d0da) or advance TSC frequency. This is useful when migrating to a host with different frequency and the guest is possibly using direct RDTSC instructions for purposes other than measuring cycles (that is, it previously calculated cycles-per-second, and uses that information which is stale after migration). qemu-x86: Set tsc_khz in kvm when supported (e7429073ed1a76518) added support for tsc_khz= option in QEMU. I am proposing the following changes so that management applications can work with this: 1) New option for tsc_khz, which is tsc_khz=host (QEMU command line option). Host means that QEMU is responsible for retrieving the TSC frequency of the host processor and use that. Management application does not have to deal with the burden. FYI, libvirt already has support for expressing a number of different TSC related config options, for support of Xen and VMWare's capabilities in this area. What we currently allow for is timer name='tsc' frequency='NNN' mode='auto|native|emulate|smpsafe'/ In this context the frequency attribute provides the HZ value to provide to the guest. - auto == Emulate if TSC is unstable, else allow native TSC access - native == Always allow native TSC access - emulate = Always emulate TSC - smpsafe == Always emulate TSC, and interlock SMP These options can be mapped into KVM if necessary (they can map to tsc_khz=XXX or to the module options (unfortunately not per-guest ATM)). Therefore it appears that this tsc_khz=auto option can be specified only if the user specifies so (it can be a per-guest flag hidden in the management configuration/manual). Sending this email to gather suggestions (or objections) to this interface. 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 :| Karen had the suggestion to remove the burden of choice from the user, which we can achieve by knowing whether or not the guest is using a paravirtual clock. The problem is that opens a can of races: Did migration happen before or after guest boot process enabled the paravirtual clock etc. I suppose leaving the option to the user is fine: if you run an obscure operating system, which does not support paravirtual clock, then it must be dealt with specialy (its in the manual, no big deal). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list