Re: [libvirt] [PATCH 3/4] qemu: parse and create -cpu ..., -kvmclock
On Mon, Jan 23, 2012 at 14:11:10 +0100, Paolo Bonzini wrote: Creating part of the -cpu command-line from something other than the cpu XML element introduces some ugliness. Well, that's what we get for creating virtual CPUID features :-) --- src/qemu/qemu_command.c| 72 ++-- tests/qemuargv2xmltest.c |4 + .../qemuxml2argv-cpu-host-kvmclock.args|4 + .../qemuxml2argv-cpu-host-kvmclock.xml | 23 ++ .../qemuxml2argv-cpu-kvmclock.args |4 + .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 24 +++ tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args |4 + tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml | 21 ++ tests/qemuxml2argvtest.c |3 + tests/qemuxml2xmltest.c|3 + 10 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvmclock.xml Thanks for creating tests right away :-) ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH v2 4/4] Allow choice of shutdown method via virsh
The 23/01/12, Daniel P. Berrange wrote: On Mon, Jan 23, 2012 at 04:41:43PM +0100, Nicolas Sebrecht wrote: The 23/01/12, Michal Privoznik wrote: +By default the hypervisor will try to pick a suitable shutdown +method. To specify an alternative method, the I--mode parameter +can specify Cacpi or Cagent. + What's the suitable shutdown method, BTW? If I don't give the --mode option, it is not clear which method will be used. That is intentional, because the choice of what is used as the default option is hypervisor depedendant and may change at will Thank you for the information but I think my point still applies. I mean, what are the options/conditions which can change this hypervisor dependent default option which may change at will? A compilation option? A qemu option? A libvirt option? The current documentation is a bit cryptic for the user unless we document _how_ is defined the suitable shutdown method. Exposing what is the current enabled method (if possible) could be interesting, too. -- Nicolas Sebrecht -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib] API to get/set custom metadata from/to domain config
On Mon, Jan 23, 2012 at 11:31:57PM +0200, Zeeshan Ali (Khattak) wrote: On Mon, Jan 23, 2012 at 7:19 PM, Christophe Fergeau cferg...@redhat.com wrote: Based on a patch from Zeeshan Ali (Khattak) zeesha...@gnome.org Looks good! I'll test it later I've only compile-tested it, if it doesn't work, don't waste time on it, just tell me to test my stuff + gvir_config_object_set_namespace(custom_xml, ns, ns_uri, TRUE); You meant to pass 'FALSE' in the last arg? Dunno, depends on what we decide to do :) I prefer FALSE, your initial patch uses TRUE so I kept that for now. + g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(object), FALSE); + g_return_val_if_fail(ns != NULL, FALSE); + g_return_val_if_fail(ns_uri != NULL, FALSE); + + namespace = xmlNewNs(object-priv-node, + (xmlChar *)ns_uri, (xmlChar *)ns); + if (namespace == NULL) + return FALSE; + if (recursive) { + set_namespace_on_tree(object-priv-node, namespace); + } else { + xmlSetNs(object-priv-node, namespace); + } You don't need the curly braces here. I know, but I like them :) I don't mind dropping them Christophe pgpZ9R1GFRlBt.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt and mingw64 x64, bad idea?
On Mon, Jan 23, 2012 at 05:20:50PM -0700, Eric Blake wrote: On 01/23/2012 03:29 PM, Marc-André Lureau wrote: Hi, I tried to update the fedora mingw package to follow the mingw64 packaging guideline and allow build for x86 and x64. But I got into build warning and errors for x86_64. (using Fedora Cross project repo: http://build1.openftd.org/fedora-cross/fedora-cross.repo, x86_64-w64-mingw32-gcc (GCC) 4.6.2 20110908 and later) The somewhat worrying error is: util/command.c:54:1: error: static assertion failed: verify (sizeof(pid_t) = sizeof(int)) /* We have quite a bit of changes to make if this doesn't hold. */ verify(sizeof(pid_t) = sizeof(int)); Oh my. I guess it makes sense - while Linux pid_t is just a (linearly-incrementing) index into an array of kernel structs, other OS's use pid_t as the actual kernel pointer to the location in physical memory where the process information is stored; so if you have 64-bit pointers, you have a 64-bit pid_t. Alas, this means that any interface where we are operating on pid_t, but passed an int, are broken. Thankfully, I don't see any interface like that in libvirt.c, and we are free to change any internal functions without breaking the ABI. Arguably, we could even change the public API, provided that we hide the change behind an #ifdef WIN64, since we have never done a release that is officially working on WIN64. That would obviously want to be something of a last resort though. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] Add test case for virHashEqual function
On 01/24/2012 06:19 AM, Michal Privoznik wrote: Therefore I am pushing this under trivial and build-breaker rules (yeah, one thing - and perhaps the only one - i like about 4.6 gcc is enhanced static analysis as I spotted warning while compiling current HEAD): Author: Michal Privoznikmpriv...@redhat.com Date: Tue Jan 24 12:09:42 2012 +0100 hashtest: Initialize variable in virHashEqual test One of latest patches (b7bcb22ce2) enhanced testing for virHashEqual. However, hash2 variable might be used uninitialized. diff --git a/tests/hashtest.c b/tests/hashtest.c index 6c45b01..441672c 100644 --- a/tests/hashtest.c +++ b/tests/hashtest.c @@ -583,7 +583,7 @@ testHashEqualCompValue(const void *value1, const void *value2) static int testHashEqual(const void *data ATTRIBUTE_UNUSED) { -virHashTablePtr hash1, hash2; +virHashTablePtr hash1, hash2 = NULL; int ret = -1; char keya[] = a; char keyb[] = b; ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] Add test case for virHashEqual function
On 18.01.2012 17:20, Stefan Berger wrote: + +static int +testHashEqual(const void *data ATTRIBUTE_UNUSED) +{ +virHashTablePtr hash1, hash2; +int ret = -1; +char keya[] = a; +char keyb[] = b; +char keyc[] = c; +char value1[] = 1; +char value2[] = 2; +char value3[] = 3; +char value4[] = 4; + +if (!(hash1 = virHashCreate(0, NULL)) || +!(hash2 = virHashCreate(0, NULL)) || Actually, if the first virHashCreate() returns NULL, hash2 remains uninitialized and we jump +virHashAddEntry(hash1, keya, value1) 0 || +virHashAddEntry(hash1, keyb, value2) 0 || +virHashAddEntry(hash1, keyc, value3) 0 || +virHashAddEntry(hash2, keya, value1) 0 || +virHashAddEntry(hash2, keyb, value2) 0) { +if (virTestGetVerbose()) { +testError(\nfailed to create hashes); +} +goto cleanup; +} over here and do free() on uninitialized pointer. + +cleanup: +virHashFree(hash1); +virHashFree(hash2); +return ret; +} Therefore I am pushing this under trivial and build-breaker rules (yeah, one thing - and perhaps the only one - i like about 4.6 gcc is enhanced static analysis as I spotted warning while compiling current HEAD): Author: Michal Privoznik mpriv...@redhat.com Date: Tue Jan 24 12:09:42 2012 +0100 hashtest: Initialize variable in virHashEqual test One of latest patches (b7bcb22ce2) enhanced testing for virHashEqual. However, hash2 variable might be used uninitialized. diff --git a/tests/hashtest.c b/tests/hashtest.c index 6c45b01..441672c 100644 --- a/tests/hashtest.c +++ b/tests/hashtest.c @@ -583,7 +583,7 @@ testHashEqualCompValue(const void *value1, const void *value2) static int testHashEqual(const void *data ATTRIBUTE_UNUSED) { -virHashTablePtr hash1, hash2; +virHashTablePtr hash1, hash2 = NULL; int ret = -1; char keya[] = a; char keyb[] = b; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Add support for QEMU guest agent control
On 23.01.2012 15:48, Michal Privoznik wrote: These patches are taken from here: https://www.redhat.com/archives/libvir-list/2011-October/msg00135.html I've just rebased and polished them. The QEMU guest agent /usr/bin/qemu-ga has some handy functions for controlling the guest, not least, shutdown/reboot and filesystem freeze/thaw. In Fedora 15/16 the semantics of the ACPI power button have been changed to suspend-to-RAM which breaks our current shutdown implementation. By adding support for the agent we gain a more predictable way to shutdown / reboot guests. NB: the code currently has the same flaw as the monitor, in so much as we wait forever for a guest agent reply. We need to add a timeout ability to the agent code diff to v1: - Eric's review taken in Daniel P. Berrange (4): QEMU guest agent support Add new virDomainShutdownFlags API Wire up QEMU agent to reboot/shutdown APIs Allow choice of shutdown method via virsh include/libvirt/libvirt.h.in | 15 + po/POTFILES.in |1 + src/Makefile.am |1 + src/driver.h |5 + src/esx/esx_driver.c | 11 +- src/libvirt.c| 68 +++- src/libvirt_public.syms |4 + src/libxl/libxl_driver.c | 12 +- src/openvz/openvz_driver.c |1 + src/qemu/qemu_agent.c| 1112 ++ src/qemu/qemu_agent.h| 69 +++ src/qemu/qemu_domain.c | 97 src/qemu/qemu_domain.h | 22 + src/qemu/qemu_driver.c | 131 - src/qemu/qemu_monitor_json.c |2 +- src/qemu/qemu_process.c | 188 +++ src/remote/remote_driver.c |1 + src/remote/remote_protocol.x |8 +- src/remote_protocol-structs |5 + src/test/test_driver.c | 11 +- src/uml/uml_driver.c | 11 +- src/vbox/vbox_tmpl.c | 11 +- src/vmware/vmware_driver.c |1 + src/xen/xen_driver.c | 14 +- src/xenapi/xenapi_driver.c | 12 +- tools/virsh.c| 47 ++- tools/virsh.pod | 12 +- 27 files changed, 1832 insertions(+), 40 deletions(-) create mode 100644 src/qemu/qemu_agent.c create mode 100644 src/qemu/qemu_agent.h Thanks Eric; I've changed the patches as you've suggested and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Add two NUMA tuning python bindings APIs
Thanks for these comments. I will refactor the loop code, rethink about other codes and make v3 patch. Guannan Ren - Original Message - From: Eric Blake ebl...@redhat.com To: Guannan Ren g...@redhat.com Cc: libvir-list@redhat.com Sent: Saturday, January 21, 2012 11:04:15 PM Subject: Re: [libvirt] [PATCH v2] Add two NUMA tuning python bindings APIs On 01/19/2012 02:16 AM, Guannan Ren wrote: *virDomainSetNumaParameters *virDomainGetNumaParameters --- python/libvirt-override-api.xml | 13 +++ python/libvirt-override.c | 186 +++ 2 files changed, 199 insertions(+), 0 deletions(-) static PyObject * +libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { +LIBVIRT_BEGIN_ALLOW_THREADS; +i_retval = virDomainGetNumaParameters(domain, params, nparams, flags); +LIBVIRT_END_ALLOW_THREADS; Why are we getting the existing parameters, instead of just blindly constructing params based on args? I think that's a waste of effort, considering that libvirt will already reject things if the user passes in unknown key names. Besides, + +if (i_retval 0) { +free(params); +return VIR_PY_INT_FAIL; +} + +/* convert to a Python tuple of long objects */ +for (i = 0; i nparams; i++) { +PyObject *key, *val; +key = libvirt_constcharPtrWrap(params[i].field); +val = PyDict_GetItem(info, key); +Py_DECREF(key); + +if (val == NULL) +continue; this says that you will silently discard any unknown keys passed in from the python code, whereas libvirt would noisily reject unknown keys. I don't think the python code should behave differently from the C code. + +switch (params[i].type) { +case VIR_TYPED_PARAM_INT: +params[i].value.i = (int)PyInt_AS_LONG(val); +break; We're starting to repeat this code sequence in several places - we ought to refactor things to share this conversion to and from python types and virTypedParameter, so that all of the glue code can share the same conversions (and fixing bugs in one will fix all of them at the same time). +static PyObject * +libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { Indentation; also, we've lately been sticking the { of a function on column 1: static PyObject * libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + +if (i_retval 0) +return VIR_PY_NONE; I think this is okay; it tells python that the C call successfully reported an error, and that the caller can then query for the last libvirt error. + +if ((params = malloc(sizeof(*params)*nparams)) == NULL) +return VIR_PY_NONE; Bad - we didn't raise any libvirt error, so the user wouldn't be able to tell why we returned NONE. Rather, we should really be calling return PyErr_NoMemory() in situations where we failed to malloc(), so that python can correctly raise a memory exception. (You were just copying-and-pasting; the problem is pervasive throughout this file). + +LIBVIRT_BEGIN_ALLOW_THREADS; +i_retval = virDomainGetNumaParameters(domain, params, nparams, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (i_retval 0) { +free(params); +return VIR_PY_NONE; +} Okay. + +/* convert to a Python tuple of long objects */ +if ((info = PyDict_New()) == NULL) { +free(params); Memory leak - params might include VIR_TYPED_PARAM_STRING contents, so we need to call virTypedParameterArrayClear before freeing params. +return VIR_PY_NONE; Bad - PyDict_New already raised the python exception for no memory, but we silently discarded it by returning NONE instead of NULL. We should really be propagating any python errors back up the call chain. +} + +for (i = 0 ; i nparams ; i++) { +PyObject *key, *val; + +switch (params[i].type) { +case VIR_TYPED_PARAM_INT: +val = PyInt_FromLong((long)params[i].value.i); +break; Again, worth refactoring code to share this loop. + +default: +free(params); Another memory leak if params had any string contents. +Py_DECREF(info); +return VIR_PY_NONE; +} + +key = libvirt_constcharPtrWrap(params[i].field); +PyDict_SetItem(info, key, val); Bad - we should be checking for failure of PyDict_SetItem, and propagating that failure back to the caller. +} +free(params); Another memory leak. +return(info); +} + -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com
[libvirt] [PATCH] src/datatypes.h: fix typo
Signed-off-by: Alon Levy al...@redhat.com --- src/datatypes.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/datatypes.h b/src/datatypes.h index 91b1bfd..47058ed 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -31,7 +31,7 @@ * VIR_CONNECT_MAGIC: * * magic value used to protect the API when pointers to connection structures - * are passed down by the uers. + * are passed down by the users. */ # define VIR_CONNECT_MAGIC 0x4F23DEAD # define VIR_IS_CONNECT(obj) ((obj) (obj)-magic==VIR_CONNECT_MAGIC) -- 1.7.8.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Add two NUMA tuning python bindings APIs
On 01/24/2012 05:07 AM, Guan Nan Ren wrote: static PyObject * +libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { +LIBVIRT_BEGIN_ALLOW_THREADS; +i_retval = virDomainGetNumaParameters(domain, params, nparams, flags); +LIBVIRT_END_ALLOW_THREADS; Why are we getting the existing parameters, instead of just blindly constructing params based on args? I think that's a waste of effort, considering that libvirt will already reject things if the user passes in unknown key names. I finally realized _why_ we do it - that's because we want to pass the correct types to libvirt.c, but python is not strongly typed. That is, if libvirt is expecting a particular named value to be VIR_TYPED_PARAM_ULLONG, but the python code passes '1', we should be able to properly convert that python value to C's 1ULL, rather than rejecting the python code for using an incorrect type, since there is not quite a notion of incorrect type in python. That means that either we hard-code the list of all known parameter names and their type, or we make things done via a runtime query by getting all parameters to learn their types before formatting the user's settings back into something that libvirt will parse. It may also mean that libvirt itself should be a bit nicer about things - now that we have virTypedParam.c, maybe that function should be taught to do some type conversions (if the user passes in VIR_TYPED_PARAM_INT, but the code wanted VIR_TYPED_PARAM_ULLONG, libvirt could do the type conversion on the user's behalf); if libvirt is made smarter, then the python glue code might be simpler. But there's still the issue of back-compat - a newer python library talking to an older libvirt that didn't do automatic type conversion would still be stuck needing to query for types first. -- 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] src/datatypes.h: fix typo
On 24.01.2012 13:45, Alon Levy wrote: Signed-off-by: Alon Levy al...@redhat.com --- src/datatypes.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/datatypes.h b/src/datatypes.h index 91b1bfd..47058ed 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -31,7 +31,7 @@ * VIR_CONNECT_MAGIC: * * magic value used to protect the API when pointers to connection structures - * are passed down by the uers. + * are passed down by the users. */ # define VIR_CONNECT_MAGIC 0x4F23DEAD # define VIR_IS_CONNECT(obj) ((obj) (obj)-magic==VIR_CONNECT_MAGIC) ACK pushed. Thanks for catching that. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] Added RSS information gathering into qemudGetProcessInfo
One more parameter added into the function parsing /proc/pid/stat and the call of the function is fixed as well. --- v2: - correction of the format in fscanf in qemudGetProcessInfo src/qemu/qemu_driver.c | 25 ++--- 1 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 608e82a..6600afd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1049,12 +1049,13 @@ cleanup: static int -qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, -int tid) +qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, +int pid, int tid) { char *proc; FILE *pidinfo; unsigned long long usertime, systime; +long rss; int cpu; int ret; @@ -1071,6 +1072,8 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, *cpuTime = 0; if (lastCpu) *lastCpu = 0; +if (vm_rss) +*vm_rss = 0; VIR_FREE(proc); return 0; } @@ -1082,10 +1085,10 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, /* pid - stime */ %*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu /* cutime - endcode */ - %*d %*d %*d %*d %*d %*u %*u %*d %*u %*u %*u %*u + %*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u /* startstack - processor */ %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d, - usertime, systime, cpu) != 3) { + usertime, systime, rss, cpu) != 4) { VIR_FORCE_FCLOSE(pidinfo); VIR_WARN(cannot parse process status data); errno = -EINVAL; @@ -1102,9 +1105,16 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, if (lastCpu) *lastCpu = cpu; +/* We got pages + * We want kiloBytes + * _SC_PAGESIZE is page size in Bytes + * So calculate, but first lower the pagesize so we don't get overflow */ +if (vm_rss) +*vm_rss = rss * (sysconf(_SC_PAGESIZE) 10); -VIR_DEBUG(Got status for %d/%d user=%llu sys=%llu cpu=%d, - pid, tid, usertime, systime, cpu); + +VIR_DEBUG(Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld, + pid, tid, usertime, systime, cpu, rss); VIR_FORCE_FCLOSE(pidinfo); @@ -2066,7 +2076,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, if (!virDomainObjIsActive(vm)) { info-cpuTime = 0; } else { -if (qemudGetProcessInfo((info-cpuTime), NULL, vm-pid, 0) 0) { +if (qemudGetProcessInfo((info-cpuTime), NULL, NULL, vm-pid, 0) 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, %s, _(cannot read cputime for domain)); goto cleanup; @@ -3648,6 +3658,7 @@ qemudDomainGetVcpus(virDomainPtr dom, if (priv-vcpupids != NULL qemudGetProcessInfo((info[i].cpuTime), (info[i].cpu), +NULL, vm-pid, priv-vcpupids[i]) 0) { virReportSystemError(errno, %s, -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] qemu: get arch name from cpu element
On Mon, Jan 23, 2012 at 17:21:01 +0100, Paolo Bonzini wrote: On 01/23/2012 04:52 PM, Jiri Denemark wrote: I think we could just set cpu-model even if the model used in qemu command line was {qemu,kvm}{32,64}. That would probably mean we need to add some of the models in cpu_map.xml, though. Actually I was doing that on purpose, not just for laziness. :) The idea is that, if you were using -cpu just to set -cpu {qemu,kvm}{32,64},-kvmclock, you will not get an irrelevant cpumodelqemu32/model/cpu in domxml-from-native output. Yeah, I understand why you did so, however, you just discard {qemu,kvm}{32,64} models even if there were more flags than just +-kvmclock specified, don't you? Anyway, cpumodelqemu32/model/cpu doesn't seem irrelevant to me, it just describes the guest visible hardware. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] Added RSS reporting
Added RSS information gathering into qemuMemoryStats into qemu driver and the reporting into virsh dommemstat. --- v2: - fixed sign for the ret variable (can be negative) include/libvirt/libvirt.h.in |7 ++- src/qemu/qemu_driver.c | 23 ++- tools/virsh.c|2 ++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..d5ef061 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -884,11 +884,16 @@ typedef enum { /* Current balloon value (in KB). */ VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON = 6, + +/* Resident Set Size of the process running the domain. This value + * is in kB */ +VIR_DOMAIN_MEMORY_STAT_RSS = 7, + /* * The number of statistics supported by this version of the interface. * To add new statistics, add them to the enum and increase this value. */ -VIR_DOMAIN_MEMORY_STAT_NR = 7, +VIR_DOMAIN_MEMORY_STAT_NR = 8, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6600afd..20d3d84 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7978,7 +7978,7 @@ qemudDomainMemoryStats (virDomainPtr dom, { struct qemud_driver *driver = dom-conn-privateData; virDomainObjPtr vm; -unsigned int ret = -1; +int ret = -1; virCheckFlags(0, -1); @@ -7997,14 +7997,27 @@ qemudDomainMemoryStats (virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) 0) goto cleanup; -if (virDomainObjIsActive(vm)) { +if (!virDomainObjIsActive(vm)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +%s, _(domain is not running)); +} else { qemuDomainObjPrivatePtr priv = vm-privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetMemoryStats(priv-mon, stats, nr_stats); qemuDomainObjExitMonitor(driver, vm); -} else { -qemuReportError(VIR_ERR_OPERATION_INVALID, -%s, _(domain is not running)); + +if (ret = 0 ret nr_stats) { +long rss; +if (qemudGetProcessInfo(NULL, NULL, rss, vm-pid, 0) 0) { +qemuReportError(VIR_ERR_OPERATION_FAILED, %s, +_(cannot get RSS for domain)); +} else { +stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; +stats[ret].val = rss; +ret++; +} + +} } if (qemuDomainObjEndJob(driver, vm) == 0) diff --git a/tools/virsh.c b/tools/virsh.c index d635b56..d5bbabf 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1871,6 +1871,8 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) vshPrint (ctl, available %llu\n, stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON) vshPrint (ctl, actual %llu\n, stats[i].val); +if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_RSS) +vshPrint (ctl, rss %llu\n, stats[i].val); } virDomainFree(dom); -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] qemu: get arch name from cpu element
On 01/24/2012 02:28 PM, Jiri Denemark wrote: The idea is that, if you were using -cpu just to set -cpu {qemu,kvm}{32,64},-kvmclock, you will not get an irrelevant cpumodelqemu32/model/cpu in domxml-from-native output. Yeah, I understand why you did so, however, you just discard {qemu,kvm}{32,64} models even if there were more flags than just +-kvmclock specified, don't you? Right, that gives Non-empty feature list specified without CPU model when you later start the domain. I'll fix this. Anyway,cpumodelqemu32/model/cpu doesn't seem irrelevant to me, it just describes the guest visible hardware. Yeah, but arch=i686 says the same (kvm32/kvm64 shouldn't be considered by this code, only qemu32/qemu64). Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH][RFC] adding a title to the domain description informations
The idea is that currently we have only the domain name usable as a description for the domain. It is not really a good human readable identifier, as the kind of string allowed is limited (no space for example). The idea would then be to extend the existing description field in the domain XML to keep 40 or less character string to describe a domain and provide that information later for example in an extended virsh list command or for other interfaces. While the idea is simple, see attached patch for this, it becomes more complex when one tries to make accessors to set/get that title for a domain, since it's mutable and possibly could be coming from the hypervisor itself (is there anything like this in VMWare or VirtualBox?) it should to be implemented down at the driver level. Is that worth the effort ? If we go that route should we do this for other objects (network, storage, etc ...) too in the end ? here is a basic patch for just the XML side to give an idea, but adding APIs is far more work. Opinions ? Daniel Provide a title attribute on domain descriptions Allow to provide a title attribute to domain descriptions useful when presenting the domain in an user interface. diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de9b480..ad5ffb0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -58,7 +58,13 @@ ddThe content of the codedescription/code element provides a human readable description of the virtual machine. This data is not used by libvirt in any way, it can contain any information the user - wants. span class=sinceSince 0.7.2/span/dd + wants. span class=sinceSince 0.7.2/span. It is also possible + to provide an human readable title for the virtual machine, by + providing it as a codetitlecode attribute on the + codedescription/code element, this can be useful to provide an + human description when listing virtual machines (span class=since + Since 0.9.10/span)/dd + /dl h3a name=elementsOSOperating system booting/a/h3 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2041dfb..78ba67c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -10,6 +10,11 @@ -- define name=description element name=description + optional +attribute name=title + ref name=title-value/ +/attribute + /optional text/ /element /define @@ -3115,4 +3120,9 @@ /element empty/ /define + define name=title-value +data type=string + param name=pattern.{0,40}/param +/data + /define /grammar diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f97014e..0e1ff44 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1478,6 +1478,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def-cpumask); VIR_FREE(def-emulator); VIR_FREE(def-description); +VIR_FREE(def-title); virBlkioDeviceWeightArrayClear(def-blkio.devices, def-blkio.ndevices); @@ -7092,6 +7093,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* Extract documentation if present */ def-description = virXPathString(string(./description[1]), ctxt); +def-title = virXPathString(string(./description[1]/@title), ctxt); /* analysis of security label, done early even though we format it * late, so devices can refer to this for defaults */ @@ -11417,8 +11419,16 @@ virDomainDefFormatInternal(virDomainDefPtr def, virUUIDFormat(uuid, uuidstr); virBufferAsprintf(buf, uuid%s/uuid\n, uuidstr); -virBufferEscapeString(buf, description%s/description\n, - def-description); +if (def-title || def-description) { +virBufferAddLit(buf, description); +if (def-title) +virBufferEscapeString(buf, title='%s', def-title); +if (def-description) +virBufferEscapeString(buf, %s/description\n, + def-description); +else +virBufferAddLit(buf, /); +} virBufferAsprintf(buf, memory%lu/memory\n, def-mem.max_balloon); virBufferAsprintf(buf, currentMemory%lu/currentMemory\n, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b121f9c..45d70e6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1421,6 +1421,7 @@ struct _virDomainDef { unsigned char uuid[VIR_UUID_BUFLEN]; char *name; char *description; +char *title; struct { unsigned int weight; -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH][RFC] adding a title to the domain description informations
On 01/24/2012 07:18 AM, Daniel Veillard wrote: The idea is that currently we have only the domain name usable as a description for the domain. It is not really a good human readable identifier, as the kind of string allowed is limited (no space for example). The idea would then be to extend the existing description field in the domain XML to keep 40 or less character string to describe a domain and provide that information later for example in an extended virsh list command or for other interfaces. While the idea is simple, see attached patch for this, it becomes more complex when one tries to make accessors to set/get that title for a domain, since it's mutable and possibly could be coming from the hypervisor itself (is there anything like this in VMWare or VirtualBox?) it should to be implemented down at the driver level. Is that worth the effort ? If we go that route should we do this for other objects (network, storage, etc ...) too in the end ? here is a basic patch for just the XML side to give an idea, but adding APIs is far more work. Opinions ? Looks like this is more or less identical to Peter's proposal to add a title element: https://www.redhat.com/archives/libvir-list/2012-January/msg00710.html so I'm all for the idea (not sure whether your patch or Peter's is better for the idea). As far as I know, the title is _not_ coming from the hypervisor itself, nor is it guest-visible; it is merely metadata from the host's perspective. -- 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][RFC] adding a title to the domain description informations
On Tue, Jan 24, 2012 at 08:06:31AM -0700, Eric Blake wrote: On 01/24/2012 07:18 AM, Daniel Veillard wrote: The idea is that currently we have only the domain name usable as a description for the domain. It is not really a good human readable identifier, as the kind of string allowed is limited (no space for example). The idea would then be to extend the existing description field in the domain XML to keep 40 or less character string to describe a domain and provide that information later for example in an extended virsh list command or for other interfaces. While the idea is simple, see attached patch for this, it becomes more complex when one tries to make accessors to set/get that title for a domain, since it's mutable and possibly could be coming from the hypervisor itself (is there anything like this in VMWare or VirtualBox?) it should to be implemented down at the driver level. Is that worth the effort ? If we go that route should we do this for other objects (network, storage, etc ...) too in the end ? here is a basic patch for just the XML side to give an idea, but adding APIs is far more work. Opinions ? Looks like this is more or less identical to Peter's proposal to add a title element: https://www.redhat.com/archives/libvir-list/2012-January/msg00710.html so I'm all for the idea (not sure whether your patch or Peter's is better for the idea). IMHO using a element is preferrable to an attribute, particularly since we're also adding a metadata attribute. As far as I know, the title is _not_ coming from the hypervisor itself, nor is it guest-visible; it is merely metadata from the host's perspective. 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] [RFC] virCommandProcessIO(): make poll() usage more robust
POLLIN and POLLHUP are not mutually exclusive. Currently the following seems possible: the child writes 3K to its stdout or stderr pipe, and immediately closes it. We get POLLIN|POLLHUP (I'm not sure that's possible on Linux, but SUSv4 seems to allow it). We read 1K and throw away the rest. When poll() returns and we're about to check the /revents/ member in a given array element, let's map all the revents bits to two (independent) ideas: let's attempt to read(), and let's attempt to write(). This should cover all errors, EOFs, and normal conditions; the read()/write() call should report any pending error. Under this approach, both POLLHUP and POLLERR are mapped to needs read() if we're otherwise prepared for POLLIN. POLLERR also maps to needs write() if we're otherwise prepared for POLLOUT. The rest of the mappings (POLLPRI etc.) would be easy, but probably useless for pipes. Additionally, SUSv4 doesn't appear to forbid POLLIN|POLLERR (or POLLOUT|POLLERR) set simultaneously. One could argue that the read() or write() call would return without blocking in these cases (with an error), so POLLIN / POLLOUT would be justified beside POLLERR. The code now penalizes POLLIN|POLLERR differently from plain POLLERR. The former (ie. read() returning -1) is terminal and we jump to cleanup, while plain POLLERR masks only the affected file descriptor for the future. Let's unify those. Build tested only. Please keep me CC'd, I'm not subscribed. Thanks. Signed-off-by: Laszlo Ersek ler...@redhat.com --- src/util/command.c | 17 ++--- 1 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index f05493e..dc3cfc5 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1710,7 +1710,7 @@ virCommandProcessIO(virCommandPtr cmd) } for (i = 0; i nfds ; i++) { -if (fds[i].revents POLLIN +if (fds[i].revents (POLLIN | POLLHUP | POLLERR) (fds[i].fd == errfd || fds[i].fd == outfd)) { char data[1024]; char **buf; @@ -1751,7 +1751,7 @@ virCommandProcessIO(virCommandPtr cmd) } } -if (fds[i].revents POLLOUT +if (fds[i].revents (POLLOUT | POLLERR) fds[i].fd == infd) { int done; @@ -1777,19 +1777,6 @@ virCommandProcessIO(virCommandPtr cmd) } } } - -if (fds[i].revents (POLLHUP | POLLERR)) { -if (fds[i].fd == errfd) { -VIR_DEBUG(hangup on stderr); -errfd = -1; -} else if (fds[i].fd == outfd) { -VIR_DEBUG(hangup on stdout); -outfd = -1; -} else { -VIR_DEBUG(hangup on stdin); -infd = -1; -} -} } } -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol
- Original Message - From: Jiri Denemark jdene...@redhat.com To: libvir-list@redhat.com Sent: Monday, January 23, 2012 2:30:54 PM Subject: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol We already provide ways to detect when a domain has been paused as a result of I/O error, but there was no way of getting the exact error or even the device that experienced it. This new API may be used for both. --- include/libvirt/libvirt.h.in | 19 +++ src/driver.h |6 src/libvirt.c| 53 ++ src/libvirt_public.syms |5 src/remote/remote_driver.c |1 + src/remote/remote_protocol.x | 13 +- src/remote_protocol-structs |9 +++ 7 files changed, 105 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..2c3423a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count); +/** + * virDomainIoError: + * + * Disk I/O error. + */ +typedef enum { +VIR_DOMAIN_IOERROR_NONE = 0, /* no error */ +VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */ +VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */ + +#ifdef VIR_ENUM_SENTINELS +VIR_DOMAIN_IOERROR_LAST +#endif +} virDomainIoError; + Hi Jiri, how do we detect an EIO error? We should need an additional error type here? -- Federico -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu_agent: Create file system freeze and thaw functions
These functions simply issue command to guest agent which should freeze or unfreeze all file systems within guest. --- src/qemu/qemu_agent.c | 68 + src/qemu/qemu_agent.h |3 ++ 2 files changed, 71 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 6a7c7b3..3a5cc4b 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1110,3 +1110,71 @@ int qemuAgentShutdown(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +/* + * qemuAgentFSFreeze: + * @mon: Agent + * + * Issue guest-fsfreeze-freeze command to guest agent, + * which freezes all mounted file systems and returns + * number of frozen file systems on success. + * + * Returns: number of file system frozen on success, + * -1 on error. + */ +int qemuAgentFSFreeze(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-freeze, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd, reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +virJSONValueObjectGetNumberInt(reply, return, ret); + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + +/* + * qemuAgentFSThaw: + * @mon: Agent + * + * Issue guest-fsfreeze-thaw command to guest agent, + * which unfreezes all mounted file systems and returns + * number of thawed file systems on success. + * + * Returns: number of file system thawed on success, + * -1 on error. + */ +int qemuAgentFSThaw(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-thaw, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd, reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +virJSONValueObjectGetNumberInt(reply, return, ret); + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 93c2ae7..df59ef7 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -66,4 +66,7 @@ typedef enum { int qemuAgentShutdown(qemuAgentPtr mon, qemuAgentShutdownMode mode); +int qemuAgentFSFreeze(qemuAgentPtr mon); +int qemuAgentFSThaw(qemuAgentPtr mon); + #endif /* __QEMU_AGENT_H__ */ -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
With this flag, virDomainSnapshotCreate will use fs-freeze and fs-thaw guest agent commands to quiesce guest's disks. --- include/libvirt/libvirt.h.in |4 ++ src/libvirt.c|6 ++ src/qemu/qemu_driver.c | 118 - 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5e6e488..182065d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3125,6 +3125,10 @@ typedef enum { system checkpoint */ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1 5), /* reuse any existing external files */ +VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = (1 6), /* use guest agent to + quiesce all mounted + file systems within + the domain */ } virDomainSnapshotCreateFlags; /* Take a snapshot of the current VM state */ diff --git a/src/libvirt.c b/src/libvirt.c index 96ad3d5..d9b8d88 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16602,6 +16602,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * inconsistent (as if power had been pulled), and specifying this * with the VIR_DOMAIN_SNAPSHOT_CREATE_HALT flag risks data loss. * + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, then the + * libvirt will attempt to use guest agent to freeze and thaw all + * file systems in use within domain OS. However, if the guest agent + * is not present, an error is trowed. Moreover, this flags requires + * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well. + * * By default, if the snapshot involves external files, and any of the * destination files already exist as a regular file, the snapshot is * rejected to avoid losing contents of those files. However, if diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c81289a..14ad30b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9461,6 +9461,56 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) return 1; } +static int +qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, + virDomainObjPtr vm) { +qemuDomainObjPrivatePtr priv = vm-privateData; +int freezed; + +if (priv-agentError) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(QEMU guest agent is not + available due to an error)); +return -1; +} +if (!priv-agent) { +qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, +_(QEMU guest agent is not configured)); +return -1; +} + +qemuDomainObjEnterAgent(driver, vm); +freezed = qemuAgentFSFreeze(priv-agent); +qemuDomainObjExitAgent(driver, vm); + +return freezed; +} + +static int +qemuDomainSnapshotFSThaw(struct qemud_driver *driver, + virDomainObjPtr vm) { +qemuDomainObjPrivatePtr priv = vm-privateData; +int thawed; + +if (priv-agentError) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(QEMU guest agent is not + available due to an error)); +return -1; +} +if (!priv-agent) { +qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, +_(QEMU guest agent is not configured)); +return -1; +} + +qemuDomainObjEnterAgent(driver, vm); +thawed = qemuAgentFSThaw(priv-agent); +qemuDomainObjExitAgent(driver, vm); + +return thawed; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotCreateInactive(struct qemud_driver *driver, @@ -9493,6 +9543,13 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { +if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) +(qemuDomainSnapshotFSFreeze(driver, vm) 0)) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(Unable to freeze domain's file systems)); +goto endjob; +} + /* savevm monitor command pauses the domain emitting an event which * confuses libvirt since it's not notified when qemu resumes the * domain. Thus we stop and start CPUs ourselves. @@ -9532,13 +9589,21 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, } cleanup: -if (resume virDomainObjIsActive(vm) -qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_NONE) 0 -virGetLastError() == NULL) { -qemuReportError(VIR_ERR_OPERATION_FAILED, %s, -_(resuming after
[libvirt] [PATCH 0/3] Use guest agent to quiesce disks
Since we have qemu guest agent support in libvirt, we can start wiring up some things that GA already knows how to do. One of them is file system freeze and thaw. Domain snapshots can profit from this functionality. Michal Privoznik (3): qemu_agent: Create file system freeze and thaw functions snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag include/libvirt/libvirt.h.in |4 ++ src/libvirt.c|6 ++ src/qemu/qemu_agent.c| 68 src/qemu/qemu_agent.h|3 + src/qemu/qemu_driver.c | 118 - tools/virsh.c|6 ++ tools/virsh.pod | 14 - 7 files changed, 202 insertions(+), 17 deletions(-) -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
to cmdSnapshotCreate and cmdSnapshotCreateAs. --- tools/virsh.c |6 ++ tools/virsh.pod | 14 -- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5560988..41c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14662,6 +14662,7 @@ static const vshCmdOptDef opts_snapshot_create[] = { {halt, VSH_OT_BOOL, 0, N_(halt domain after snapshot is created)}, {disk-only, VSH_OT_BOOL, 0, N_(capture disk state but not vm state)}, {reuse-external, VSH_OT_BOOL, 0, N_(reuse any existing external files)}, +{quiesce, VSH_OT_BOOL, 0, N_(quiesce guest's file systems)}, {NULL, 0, 0, NULL} }; @@ -14686,6 +14687,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, reuse-external)) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; +if (vshCommandOptBool(cmd, quiesce)) +flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl-conn)) goto cleanup; @@ -14795,6 +14798,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { {halt, VSH_OT_BOOL, 0, N_(halt domain after snapshot is created)}, {disk-only, VSH_OT_BOOL, 0, N_(capture disk state but not vm state)}, {reuse-external, VSH_OT_BOOL, 0, N_(reuse any existing external files)}, +{quiesce, VSH_OT_BOOL, 0, N_(quiesce guest's file systems)}, {diskspec, VSH_OT_ARGV, 0, N_(disk attributes: disk[,snapshot=type][,driver=type][,file=name])}, {NULL, 0, 0, NULL} @@ -14820,6 +14824,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, reuse-external)) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; +if (vshCommandOptBool(cmd, quiesce)) +flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl-conn)) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index 72c6d8f..b1f75de 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2046,7 +2046,8 @@ used to represent properties of snapshots. =over 4 =item Bsnapshot-create Idomain [Ixmlfile] {[I--redefine [I--current]] -| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external]} +| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external] +[I--quiesce]} Create a snapshot for domain Idomain with the properties specified in Ixmlfile. Normally, the only properties settable for a domain snapshot @@ -2088,6 +2089,10 @@ external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I--quiesce is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I--disk-only to be passed as well. + Existence of snapshot metadata will prevent attempts to Bundefine a persistent domain. However, for transient domains, snapshot metadata is silently lost when the domain quits running (whether @@ -2095,7 +2100,8 @@ by command such as Bdestroy or by internal guest action). =item Bsnapshot-create-as Idomain {[I--print-xml] | [I--no-metadata] [I--halt] [I--reuse-existing]} [Iname] -[Idescription] [I--disk-only [[I--diskspec] Bdiskspec]...] +[Idescription] [I--disk-only [I--quiesce] +[[I--diskspec] Bdiskspec]...] Create a snapshot for domain Idomain with the given name and description; if either value is omitted, libvirt will choose a @@ -2123,6 +2129,10 @@ option requests an external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I--quiesce is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I--disk-only to be passed as well. + If I--no-metadata is specified, then the snapshot data is created, but any metadata is immediately discarded (that is, libvirt does not treat the snapshot as current, and cannot revert to the snapshot -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol
On Tue, Jan 24, 2012 at 11:57:23 -0500, Federico Simoncelli wrote: +/** + * virDomainIoError: + * + * Disk I/O error. + */ +typedef enum { +VIR_DOMAIN_IOERROR_NONE = 0, /* no error */ +VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */ +VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */ + +#ifdef VIR_ENUM_SENTINELS +VIR_DOMAIN_IOERROR_LAST +#endif +} virDomainIoError; + Hi Jiri, how do we detect an EIO error? We should need an additional error type here? We could certainly add more error codes but doing so only makes sense when there is a hypervisor that can report them. Current qemu has three io-status values: ok, nospace, failed and these values are mapped to VIR_DOMAIN_IOERROR_NONE, VIR_DOMAIN_IOERROR_NO_SPACE, and VIR_DOMAIN_IOERROR_UNSPEC. I believe that EIO would be reported as failed by qemu and thus VIR_DOMAIN_IOERROR_UNSPEC by libvirt. It's possible that VIR_DOMAIN_IOERROR_UNSPEC is not the best name, though :-) Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu_agent: Create file system freeze and thaw functions
On Tue, Jan 24, 2012 at 18:37:43 +0100, Michal Privoznik wrote: These functions simply issue command to guest agent which should freeze or unfreeze all file systems within guest. --- src/qemu/qemu_agent.c | 68 + src/qemu/qemu_agent.h |3 ++ 2 files changed, 71 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 6a7c7b3..3a5cc4b 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1110,3 +1110,71 @@ int qemuAgentShutdown(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +/* + * qemuAgentFSFreeze: + * @mon: Agent + * + * Issue guest-fsfreeze-freeze command to guest agent, + * which freezes all mounted file systems and returns + * number of frozen file systems on success. + * + * Returns: number of file system frozen on success, + * -1 on error. + */ +int qemuAgentFSFreeze(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-freeze, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd, reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +virJSONValueObjectGetNumberInt(reply, return, ret); Doesn't the above function need to be checked for errors? That is, what if there's no return key in the reply? In json monitor code, we usually have something like the following code: if (virJSONValueObjectGetNumberInt(reply, return, ret) 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, _(guest-fsfreeze-freeze reply was missing return data)); } + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + +/* + * qemuAgentFSThaw: + * @mon: Agent + * + * Issue guest-fsfreeze-thaw command to guest agent, + * which unfreezes all mounted file systems and returns + * number of thawed file systems on success. + * + * Returns: number of file system thawed on success, + * -1 on error. + */ +int qemuAgentFSThaw(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-thaw, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd, reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +virJSONValueObjectGetNumberInt(reply, return, ret); And similar check for this line as well. + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 93c2ae7..df59ef7 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -66,4 +66,7 @@ typedef enum { int qemuAgentShutdown(qemuAgentPtr mon, qemuAgentShutdownMode mode); +int qemuAgentFSFreeze(qemuAgentPtr mon); +int qemuAgentFSThaw(qemuAgentPtr mon); + #endif /* __QEMU_AGENT_H__ */ -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] Add support for sVirt in the LXC driver
On 01/11/2012 09:33 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com For the sake of backwards compat, LXC guests are *not* confined by default. This is because it is not practical to dynamically relabel containers using large filesystem trees. Applications can create confined containers though, by giving suitable XML configs * src/Makefile.am: Link libvirt_lxc to security drivers * src/lxc/libvirtd_lxc.aug, src/lxc/lxc_conf.h, src/lxc/lxc_conf.c, src/lxc/lxc.conf, src/lxc/test_libvirtd_lxc.aug: Config file handling for security driver * src/lxc/lxc_driver.c: Wire up security driver functions * src/lxc/lxc_controller.c: Add a '--security' flag to specify which security driver to activate * src/lxc/lxc_container.c, src/lxc/lxc_container.h: Set the process label just before exec'ing init. --- +++ b/src/lxc/lxc.conf @@ -11,3 +11,21 @@ # This is disabled by default, uncomment below to enable it. # # log_with_libvirtd = 1 + + +# The default security driver is SELinux. If SELinux is disabled +# on the host, then the security driver will automatically disable +# itself. If you wish to disable QEMU SELinux security driver while +# leaving SELinux enabled for the host in general, then set this +# to 'none' instead. +# +# security_driver = selinux + +# If set to non-zero, then the default security labelling Same question as 5/7 about whether to prefer US spelling of labeling. +# will make guests confined. If set to zero, then guests +# will be unconfined by default. Defaults to zero +# security_default_confined = 1 + +# If set to non-zero, then attempts to create unconfined +# guests will be blocked. Defaults to zero. Consistency - one description ended with '.', the other did not. Back to the 5/7 question of whether this should be spelled out as 'zero' or listed as '0'. +# security_require_confined = 1 \ No newline at end of file 'make syntax-check' wasn't happy: prohibit_empty_lines_at_EOF src/lxc/lxc.conf maint.mk: empty line(s) or no newline at EOF @@ -1598,6 +1625,12 @@ lxcBuildControllerCmd(lxc_driver_t *driver, virCommandAddArgFormat(cmd, %d, ttyFDs[i]); virCommandPreserveFD(cmd, ttyFDs[i]); } + +if (driver-securityDriverName) { +virCommandAddArg(cmd, --security); +virCommandAddArg(cmd, driver-securityDriverName); +} Is it worth the shorter: if (driver-securityDriverName) virCommandAddArgPair(cmd, --security, driver-securityDriverName); + +static int lxcNodeGetSecurityModel(virConnectPtr conn, + virSecurityModelPtr secmodel) +{ + +p = driver-caps-host.secModel.model; +if (strlen(p) = VIR_SECURITY_MODEL_BUFLEN-1) { +lxcError(VIR_ERR_INTERNAL_ERROR, + _(security model string exceeds max %d bytes), + VIR_SECURITY_MODEL_BUFLEN-1); +ret = -1; +goto cleanup; +} +strcpy(secmodel-model, p); Rather than doing length checks and then strcpy, wouldn't it be better to use virStrncpy? (Twice in this function). @@ -3859,6 +4051,8 @@ static virDriver lxcDriver = { .domainGetBlkioParameters = lxcDomainGetBlkioParameters, /* 0.9.8 */ .domainGetInfo = lxcDomainGetInfo, /* 0.4.2 */ .domainGetState = lxcDomainGetState, /* 0.9.2 */ +.domainGetSecurityLabel = lxcDomainGetSecurityLabel, /* 0.9.4 */ +.nodeGetSecurityModel = lxcNodeGetSecurityModel, /* 0.9.4 */ You've been sitting on this series for a while, now :) 0.9.10, not 0.9.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 1/3] qemu_agent: Create file system freeze and thaw functions
On 24.01.2012 19:21, Jiri Denemark wrote: On Tue, Jan 24, 2012 at 18:37:43 +0100, Michal Privoznik wrote: + +if (qemuAgentCommand(mon, cmd, reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +virJSONValueObjectGetNumberInt(reply, return, ret); Doesn't the above function need to be checked for errors? That is, what if there's no return key in the reply? In json monitor code, we usually have something like the following code: if (virJSONValueObjectGetNumberInt(reply, return, ret) 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, _(guest-fsfreeze-freeze reply was missing return data)); } In fact this check is done in qemuAgentCheckError(); which reports error as well. But I agree I should check if returned value is integer and throw an error. I have squashed this into my local branch: diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 3a5cc4b..9df5546 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1137,7 +1137,10 @@ int qemuAgentFSFreeze(qemuAgentPtr mon) qemuAgentCheckError(cmd, reply) 0) goto cleanup; -virJSONValueObjectGetNumberInt(reply, return, ret); +if (virJSONValueObjectGetNumberInt(reply, return, ret) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(malformed return value)); +} cleanup: virJSONValueFree(cmd); @@ -1171,7 +1174,10 @@ int qemuAgentFSThaw(qemuAgentPtr mon) qemuAgentCheckError(cmd, reply) 0) goto cleanup; -virJSONValueObjectGetNumberInt(reply, return, ret); +if (virJSONValueObjectGetNumberInt(reply, return, ret) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(malformed return value)); +} cleanup: virJSONValueFree(cmd); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
On Tue, Jan 24, 2012 at 18:37:44 +0100, Michal Privoznik wrote: With this flag, virDomainSnapshotCreate will use fs-freeze and fs-thaw guest agent commands to quiesce guest's disks. --- include/libvirt/libvirt.h.in |4 ++ src/libvirt.c|6 ++ src/qemu/qemu_driver.c | 118 - 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5e6e488..182065d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3125,6 +3125,10 @@ typedef enum { system checkpoint */ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1 5), /* reuse any existing external files */ +VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = (1 6), /* use guest agent to + quiesce all mounted + file systems within + the domain */ Do we also want to add another flag that would make quiescing optional? That is, use it if it's available but don't fail if it's not. } virDomainSnapshotCreateFlags; /* Take a snapshot of the current VM state */ diff --git a/src/libvirt.c b/src/libvirt.c index 96ad3d5..d9b8d88 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16602,6 +16602,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * inconsistent (as if power had been pulled), and specifying this * with the VIR_DOMAIN_SNAPSHOT_CREATE_HALT flag risks data loss. * + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, then the + * libvirt will attempt to use guest agent to freeze and thaw all + * file systems in use within domain OS. However, if the guest agent + * is not present, an error is trowed. Moreover, this flags requires s/trowed/thrown/; s/flags/flag/ + * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well. + * * By default, if the snapshot involves external files, and any of the * destination files already exist as a regular file, the snapshot is * rejected to avoid losing contents of those files. However, if diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c81289a..14ad30b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9461,6 +9461,56 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) return 1; } +static int +qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, + virDomainObjPtr vm) { +qemuDomainObjPrivatePtr priv = vm-privateData; +int freezed; + +if (priv-agentError) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(QEMU guest agent is not + available due to an error)); +return -1; +} +if (!priv-agent) { +qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, +_(QEMU guest agent is not configured)); +return -1; +} + +qemuDomainObjEnterAgent(driver, vm); +freezed = qemuAgentFSFreeze(priv-agent); +qemuDomainObjExitAgent(driver, vm); + +return freezed; +} + +static int +qemuDomainSnapshotFSThaw(struct qemud_driver *driver, + virDomainObjPtr vm) { +qemuDomainObjPrivatePtr priv = vm-privateData; +int thawed; + +if (priv-agentError) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(QEMU guest agent is not + available due to an error)); +return -1; +} +if (!priv-agent) { +qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, +_(QEMU guest agent is not configured)); +return -1; +} + +qemuDomainObjEnterAgent(driver, vm); +thawed = qemuAgentFSThaw(priv-agent); +qemuDomainObjExitAgent(driver, vm); + +return thawed; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotCreateInactive(struct qemud_driver *driver, @@ -9493,6 +9543,13 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { +if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) +(qemuDomainSnapshotFSFreeze(driver, vm) 0)) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(Unable to freeze domain's file systems)); This just masks useful error reported by qemuDomainSnapshotFSFreeze(). +goto endjob; +} + /* savevm monitor command pauses the domain emitting an event which * confuses libvirt since it's not notified when qemu resumes the * domain. Thus we stop and start CPUs ourselves. @@ -9532,13
[libvirt] [PATCH] lxc: export container=lxc-libvirt for systemd
Systemd detects containers based on whether they have an environment variable starting with 'container=lxc'; using a longer name fits the expectations, while also allowing detection of who created the container. Requested by Lennart Poettering, in response to https://bugs.freedesktop.org/show_bug.cgi?id=45175 * src/lxc/lxc_container.c (lxcContainerBuildInitCmd): Add another env-var. --- src/lxc/lxc_container.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index c1ec9c4..70f6908 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2011 Red Hat, Inc. + * Copyright (C) 2008-2012 Red Hat, Inc. * Copyright (C) 2008 IBM Corp. * * lxc_container.c: file description @@ -124,6 +124,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); +virCommandAddEnvString(cmd, container=lxc-libvirt); virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr); virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name); if (vmDef-os.cmdline) -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
On Tue, Jan 24, 2012 at 18:37:45 +0100, Michal Privoznik wrote: to cmdSnapshotCreate and cmdSnapshotCreateAs. --- tools/virsh.c |6 ++ tools/virsh.pod | 14 -- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5560988..41c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14662,6 +14662,7 @@ static const vshCmdOptDef opts_snapshot_create[] = { {halt, VSH_OT_BOOL, 0, N_(halt domain after snapshot is created)}, {disk-only, VSH_OT_BOOL, 0, N_(capture disk state but not vm state)}, {reuse-external, VSH_OT_BOOL, 0, N_(reuse any existing external files)}, +{quiesce, VSH_OT_BOOL, 0, N_(quiesce guest's file systems)}, {NULL, 0, 0, NULL} }; @@ -14686,6 +14687,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, reuse-external)) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; +if (vshCommandOptBool(cmd, quiesce)) +flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl-conn)) goto cleanup; @@ -14795,6 +14798,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { {halt, VSH_OT_BOOL, 0, N_(halt domain after snapshot is created)}, {disk-only, VSH_OT_BOOL, 0, N_(capture disk state but not vm state)}, {reuse-external, VSH_OT_BOOL, 0, N_(reuse any existing external files)}, +{quiesce, VSH_OT_BOOL, 0, N_(quiesce guest's file systems)}, {diskspec, VSH_OT_ARGV, 0, N_(disk attributes: disk[,snapshot=type][,driver=type][,file=name])}, {NULL, 0, 0, NULL} @@ -14820,6 +14824,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, reuse-external)) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; +if (vshCommandOptBool(cmd, quiesce)) +flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl-conn)) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index 72c6d8f..b1f75de 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2046,7 +2046,8 @@ used to represent properties of snapshots. =over 4 =item Bsnapshot-create Idomain [Ixmlfile] {[I--redefine [I--current]] -| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external]} +| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external] +[I--quiesce]} Create a snapshot for domain Idomain with the properties specified in Ixmlfile. Normally, the only properties settable for a domain snapshot @@ -2088,6 +2089,10 @@ external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I--quiesce is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I--disk-only to be passed as well. + I'm not a native speaker but I feel like libvirt will try to use guest agent suggests that the snapshot will still be created even though guest agent couldn't be used for freezing guest's file systems. I'd rather be explicit about the fact that no guest agent implies no snapshot. Existence of snapshot metadata will prevent attempts to Bundefine a persistent domain. However, for transient domains, snapshot metadata is silently lost when the domain quits running (whether @@ -2095,7 +2100,8 @@ by command such as Bdestroy or by internal guest action). =item Bsnapshot-create-as Idomain {[I--print-xml] | [I--no-metadata] [I--halt] [I--reuse-existing]} [Iname] -[Idescription] [I--disk-only [[I--diskspec] Bdiskspec]...] +[Idescription] [I--disk-only [I--quiesce] +[[I--diskspec] Bdiskspec]...] Create a snapshot for domain Idomain with the given name and description; if either value is omitted, libvirt will choose a @@ -2123,6 +2129,10 @@ option requests an external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I--quiesce is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I--disk-only to be passed as well. + And here as well. If I--no-metadata is specified, then the snapshot data is created, but any metadata is immediately discarded (that is, libvirt does not treat the snapshot as current, and cannot revert to the snapshot Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
On 24.01.2012 19:57, Jiri Denemark wrote: On Tue, Jan 24, 2012 at 18:37:45 +0100, Michal Privoznik wrote: to cmdSnapshotCreate and cmdSnapshotCreateAs. --- tools/virsh.c |6 ++ tools/virsh.pod | 14 -- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 72c6d8f..b1f75de 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2046,7 +2046,8 @@ used to represent properties of snapshots. =over 4 =item Bsnapshot-create Idomain [Ixmlfile] {[I--redefine [I--current]] -| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external]} +| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external] +[I--quiesce]} Create a snapshot for domain Idomain with the properties specified in Ixmlfile. Normally, the only properties settable for a domain snapshot @@ -2088,6 +2089,10 @@ external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I--quiesce is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +currently this requires I--disk-only to be passed as well. + I'm not a native speaker but I feel like libvirt will try to use guest agent suggests that the snapshot will still be created even though guest agent couldn't be used for freezing guest's file systems. I'd rather be explicit about the fact that no guest agent implies no snapshot. Okay, reworded in v2. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
On 24.01.2012 19:49, Jiri Denemark wrote: On Tue, Jan 24, 2012 at 18:37:44 +0100, Michal Privoznik wrote: With this flag, virDomainSnapshotCreate will use fs-freeze and fs-thaw guest agent commands to quiesce guest's disks. --- include/libvirt/libvirt.h.in |4 ++ src/libvirt.c|6 ++ src/qemu/qemu_driver.c | 118 - 3 files changed, 113 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5e6e488..182065d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3125,6 +3125,10 @@ typedef enum { system checkpoint */ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1 5), /* reuse any existing external files */ +VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = (1 6), /* use guest agent to + quiesce all mounted + file systems within + the domain */ Do we also want to add another flag that would make quiescing optional? That is, use it if it's available but don't fail if it's not. If this is ever needed we can add it then. @@ -9493,6 +9543,13 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { +if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) +(qemuDomainSnapshotFSFreeze(driver, vm) 0)) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(Unable to freeze domain's file systems)); This just masks useful error reported by qemuDomainSnapshotFSFreeze(). In fact, I just realized, this is dead code. Since we are checking for DISK_ONLY flag which implies calling qemuDomainSnapshotCreateDiskActive() we will never get this condition to evaluate as true. I'll drop it in v2. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] qemu_agent: Create file system freeze and thaw functions
These functions simply issue command to guest agent which should freeze or unfreeze all file systems within guest. --- src/qemu/qemu_agent.c | 74 + src/qemu/qemu_agent.h |3 ++ 2 files changed, 77 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 6a7c7b3..9df5546 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1110,3 +1110,77 @@ int qemuAgentShutdown(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +/* + * qemuAgentFSFreeze: + * @mon: Agent + * + * Issue guest-fsfreeze-freeze command to guest agent, + * which freezes all mounted file systems and returns + * number of frozen file systems on success. + * + * Returns: number of file system frozen on success, + * -1 on error. + */ +int qemuAgentFSFreeze(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-freeze, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd, reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +if (virJSONValueObjectGetNumberInt(reply, return, ret) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(malformed return value)); +} + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + +/* + * qemuAgentFSThaw: + * @mon: Agent + * + * Issue guest-fsfreeze-thaw command to guest agent, + * which unfreezes all mounted file systems and returns + * number of thawed file systems on success. + * + * Returns: number of file system thawed on success, + * -1 on error. + */ +int qemuAgentFSThaw(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-thaw, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd, reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +if (virJSONValueObjectGetNumberInt(reply, return, ret) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(malformed return value)); +} + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 93c2ae7..df59ef7 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -66,4 +66,7 @@ typedef enum { int qemuAgentShutdown(qemuAgentPtr mon, qemuAgentShutdownMode mode); +int qemuAgentFSFreeze(qemuAgentPtr mon); +int qemuAgentFSThaw(qemuAgentPtr mon); + #endif /* __QEMU_AGENT_H__ */ -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] Use guest agent to quiesce disks
Since we have qemu guest agent support in libvirt, we can start wiring up some things that GA already knows how to do. One of them is file system freeze and thaw. Domain snapshots can profit from this functionality. Michal Privoznik (3): qemu_agent: Create file system freeze and thaw functions snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag include/libvirt/libvirt.h.in |4 ++ src/libvirt.c|6 +++ src/qemu/qemu_agent.c| 74 +++ src/qemu/qemu_agent.h|3 + src/qemu/qemu_driver.c | 87 ++ tools/virsh.c|6 +++ tools/virsh.pod | 16 +++- 7 files changed, 186 insertions(+), 10 deletions(-) -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] snapshots: Introduce VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
With this flag, virDomainSnapshotCreate will use fs-freeze and fs-thaw guest agent commands to quiesce guest's disks. --- include/libvirt/libvirt.h.in |4 ++ src/libvirt.c|6 +++ src/qemu/qemu_driver.c | 87 ++ 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5e6e488..182065d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3125,6 +3125,10 @@ typedef enum { system checkpoint */ VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT = (1 5), /* reuse any existing external files */ +VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = (1 6), /* use guest agent to + quiesce all mounted + file systems within + the domain */ } virDomainSnapshotCreateFlags; /* Take a snapshot of the current VM state */ diff --git a/src/libvirt.c b/src/libvirt.c index 96ad3d5..722a2e2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16602,6 +16602,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) * inconsistent (as if power had been pulled), and specifying this * with the VIR_DOMAIN_SNAPSHOT_CREATE_HALT flag risks data loss. * + * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, then the + * libvirt will attempt to use guest agent to freeze and thaw all + * file systems in use within domain OS. However, if the guest agent + * is not present, an error is thrown. Moreover, this flag requires + * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well. + * * By default, if the snapshot involves external files, and any of the * destination files already exist as a regular file, the snapshot is * rejected to avoid losing contents of those files. However, if diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c81289a..ab69dca 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9461,6 +9461,56 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) return 1; } +static int +qemuDomainSnapshotFSFreeze(struct qemud_driver *driver, + virDomainObjPtr vm) { +qemuDomainObjPrivatePtr priv = vm-privateData; +int freezed; + +if (priv-agentError) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(QEMU guest agent is not + available due to an error)); +return -1; +} +if (!priv-agent) { +qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, +_(QEMU guest agent is not configured)); +return -1; +} + +qemuDomainObjEnterAgent(driver, vm); +freezed = qemuAgentFSFreeze(priv-agent); +qemuDomainObjExitAgent(driver, vm); + +return freezed; +} + +static int +qemuDomainSnapshotFSThaw(struct qemud_driver *driver, + virDomainObjPtr vm) { +qemuDomainObjPrivatePtr priv = vm-privateData; +int thawed; + +if (priv-agentError) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(QEMU guest agent is not + available due to an error)); +return -1; +} +if (!priv-agent) { +qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, +_(QEMU guest agent is not configured)); +return -1; +} + +qemuDomainObjEnterAgent(driver, vm); +thawed = qemuAgentFSThaw(priv-agent); +qemuDomainObjExitAgent(driver, vm); + +return thawed; +} + /* The domain is expected to be locked and inactive. */ static int qemuDomainSnapshotCreateInactive(struct qemud_driver *driver, @@ -9773,6 +9823,12 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { +if ((flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) +(qemuDomainSnapshotFSFreeze(driver, vm) 0)) { +/* helper reported the error */ +goto endjob; +} + /* In qemu, snapshot_blkdev on a single disk will pause cpus, * but this confuses libvirt since notifications are not given * when qemu resumes. And for multiple disks, libvirt must @@ -9840,13 +9896,20 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, } cleanup: -if (resume virDomainObjIsActive(vm) -qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_NONE) 0 -virGetLastError() == NULL) { -qemuReportError(VIR_ERR_OPERATION_FAILED, %s, -_(resuming after snapshot failed)); +if (resume virDomainObjIsActive(vm)) { +if
[libvirt] [PATCH v2 3/3] virsh: Expose new VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag
to cmdSnapshotCreate and cmdSnapshotCreateAs. --- tools/virsh.c |6 ++ tools/virsh.pod | 16 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5560988..41c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14662,6 +14662,7 @@ static const vshCmdOptDef opts_snapshot_create[] = { {halt, VSH_OT_BOOL, 0, N_(halt domain after snapshot is created)}, {disk-only, VSH_OT_BOOL, 0, N_(capture disk state but not vm state)}, {reuse-external, VSH_OT_BOOL, 0, N_(reuse any existing external files)}, +{quiesce, VSH_OT_BOOL, 0, N_(quiesce guest's file systems)}, {NULL, 0, 0, NULL} }; @@ -14686,6 +14687,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, reuse-external)) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; +if (vshCommandOptBool(cmd, quiesce)) +flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl-conn)) goto cleanup; @@ -14795,6 +14798,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { {halt, VSH_OT_BOOL, 0, N_(halt domain after snapshot is created)}, {disk-only, VSH_OT_BOOL, 0, N_(capture disk state but not vm state)}, {reuse-external, VSH_OT_BOOL, 0, N_(reuse any existing external files)}, +{quiesce, VSH_OT_BOOL, 0, N_(quiesce guest's file systems)}, {diskspec, VSH_OT_ARGV, 0, N_(disk attributes: disk[,snapshot=type][,driver=type][,file=name])}, {NULL, 0, 0, NULL} @@ -14820,6 +14824,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; if (vshCommandOptBool(cmd, reuse-external)) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT; +if (vshCommandOptBool(cmd, quiesce)) +flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; if (!vshConnectionUsability(ctl, ctl-conn)) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index 72c6d8f..b6962cf 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2046,7 +2046,8 @@ used to represent properties of snapshots. =over 4 =item Bsnapshot-create Idomain [Ixmlfile] {[I--redefine [I--current]] -| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external]} +| [I--no-metadata] [I--halt] [I--disk-only] [I--reuse-external] +[I--quiesce]} Create a snapshot for domain Idomain with the properties specified in Ixmlfile. Normally, the only properties settable for a domain snapshot @@ -2088,6 +2089,11 @@ external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I--quiesce is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +if domain has no guest agent, snapshot creation will fail. +Currently, this requires I--disk-only to be passed as well. + Existence of snapshot metadata will prevent attempts to Bundefine a persistent domain. However, for transient domains, snapshot metadata is silently lost when the domain quits running (whether @@ -2095,7 +2101,8 @@ by command such as Bdestroy or by internal guest action). =item Bsnapshot-create-as Idomain {[I--print-xml] | [I--no-metadata] [I--halt] [I--reuse-existing]} [Iname] -[Idescription] [I--disk-only [[I--diskspec] Bdiskspec]...] +[Idescription] [I--disk-only [I--quiesce] +[[I--diskspec] Bdiskspec]...] Create a snapshot for domain Idomain with the given name and description; if either value is omitted, libvirt will choose a @@ -2123,6 +2130,11 @@ option requests an external snapshot with a destination of an existing file, then the existing file is truncated and reused; otherwise, a snapshot is refused to avoid losing contents of the existing files. +If I--quiesce is specified, libvirt will try to use guest agent +to freeze and unfreeze domain's mounted file systems. However, +if domain has no guest agent, snapshot creation will fail. +Currently, this requires I--disk-only to be passed as well. + If I--no-metadata is specified, then the snapshot data is created, but any metadata is immediately discarded (that is, libvirt does not treat the snapshot as current, and cannot revert to the snapshot -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] Set a security context on /dev and /dev/pts mounts
On 01/11/2012 09:33 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com To allow the container to access /dev and /dev/pts when under sVirt, set an explicit mount option. Also set a max size on the /dev mount to prevent DOS on memory usage * src/lxc/lxc_container.c: Set /dev mount context * src/lxc/lxc_controller.c: Set /dev/pts mount context --- src/lxc/lxc_container.c | 75 +++-- src/lxc/lxc_controller.c | 43 +++--- 2 files changed, 96 insertions(+), 22 deletions(-) +} else { +#endif +/* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ +if (virAsprintf(opts, mode=755,size=65536%%%s%s%s, Ouch. size=65536% is _not_ what you want; you either want size=65536 or something like size=10%. +con ? ,context=\ : , +con ? (const char *)con : , +con ? \ : ) 0) { I would have split this: if (virAsprintf(opts, mode=755,size=65536) 0 || (con virAsprintf(opts, ,context=\%s\, (const char *)con) 0)) { +virReportOOMError(); +goto cleanup; +} +#if HAVE_SELINUX +} +#endif You don't need this second #if. That is, instead of writing: #if HAVE_SELINUX if (condition) { goto cleanup; } else { #endif stuff; #if HAVE_SELINUX } #endif I would have done: #if HAVE_SELINUX if (condition) { goto cleanup; } #endif stuff; @@ -1373,16 +1380,42 @@ lxcControllerRun(virDomainDefPtr def, goto cleanup; } -/* XXX should we support gid=X for X!=5 for distros which use - * a different gid for tty? */ -VIR_DEBUG(Mounting 'devpts' on %s, devpts); -if (mount(devpts, devpts, devpts, 0, - newinstance,ptmxmode=0666,mode=0620,gid=5) 0) { +#if HAVE_SELINUX +if (getfilecon(root-src, con) 0 +errno != ENOTSUP) { +virReportSystemError(errno, + _(Failed to query file context on %s), + root-src); +goto cleanup; +} else { +#endif +/* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ Is this comment really relative to the devpts mount point? +/* XXX should we support gid=X for X!=5 for distros which use + * a different gid for tty? */ +if (virAsprintf(opts, newinstance,ptmxmode=0666,mode=0620,gid=5%s%s%s, +con ? ,context=\ : , +con ? (const char *)con : , +con ? \ : ) 0) { +virReportOOMError(); +goto cleanup; +} +#if HAVE_SELINUX +} +#endif Same formatting nit about not needing a second #if. -- 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/7] Set a security context on /dev and /dev/pts mounts
On 01/24/2012 01:21 PM, Eric Blake wrote: +if (virAsprintf(opts, mode=755,size=65536%%%s%s%s, +con ? ,context=\ : , +con ? (const char *)con : , +con ? \ : ) 0) { I would have split this: if (virAsprintf(opts, mode=755,size=65536) 0 || (con virAsprintf(opts, ,context=\%s\, (const char *)con) 0)) { Never mind - that doesn't work; likewise, I don't think we have any guarantees about self-modifying strings such as: if (virAsprintf(opts, mode=755,size=65536) 0 || (con virAsprintf(opts, %s,context=\%s\, opts, (const char *)con) 0)) { I guess I was thinking virBufferAsprintf, where appending is indeed easier than doing it in one shot. -- 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] qemu: Emit bootindex even for direct boot
Direct boot (using kernel, initrd, and command line) is used by virt-install/virt-manager for network install. While any bootindex has no direct effect since -kernel is always first, we need it as a hint for SeaBIOS to present disks in the same order as they will be presented during normal boot. --- src/qemu/qemu_command.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aaccf62..9b69ad0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4334,8 +4334,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) { int bootCD = 0, bootFloppy = 0, bootDisk = 0; -if ((qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex) -!def-os.kernel) { +if ((qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) { /* bootDevs will get translated into either bootindex=N or boot=on * depending on what qemu supports */ for (i = 0 ; i def-os.nBootDevs ; i++) { -- 1.7.8.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Emit bootindex even for direct boot
On 01/24/2012 03:53 PM, Jiri Denemark wrote: Direct boot (using kernel, initrd, and command line) is used by virt-install/virt-manager for network install. While any bootindex has no direct effect since -kernel is always first, we need it as a hint for SeaBIOS to present disks in the same order as they will be presented during normal boot. --- src/qemu/qemu_command.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aaccf62..9b69ad0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4334,8 +4334,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) { int bootCD = 0, bootFloppy = 0, bootDisk = 0; -if ((qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex) -!def-os.kernel) { +if ((qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) { 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 v2 1/3] qemu_agent: Create file system freeze and thaw functions
I am happy that you provide the patches. These functions simply issue command to guest agent which should freeze or unfreeze all file systems within guest. --- src/qemu/qemu_agent.c | 74 + src/qemu/qemu_agent.h |3 ++ 2 files changed, 77 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 6a7c7b3..9df5546 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1110,3 +1110,77 @@ int qemuAgentShutdown(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +/* + * qemuAgentFSFreeze: + * @mon: Agent + * + * Issue guest-fsfreeze-freeze command to guest agent, + * which freezes all mounted file systems and returns + * number of frozen file systems on success. + * + * Returns: number of file system frozen on success, + * -1 on error. + */ +int qemuAgentFSFreeze(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-freeze, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd,reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +if (virJSONValueObjectGetNumberInt(reply, return,ret) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(malformed return value)); +} + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + +/* + * qemuAgentFSThaw: + * @mon: Agent + * + * Issue guest-fsfreeze-thaw command to guest agent, + * which unfreezes all mounted file systems and returns + * number of thawed file systems on success. + * + * Returns: number of file system thawed on success, + * -1 on error. + */ +int qemuAgentFSThaw(qemuAgentPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand(guest-fsfreeze-thaw, NULL); + +if (!cmd) +return -1; + +if (qemuAgentCommand(mon, cmd,reply) 0 || +qemuAgentCheckError(cmd, reply) 0) +goto cleanup; + +if (virJSONValueObjectGetNumberInt(reply, return,ret) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(malformed return value)); +} + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} But qemuAgentFSFreeze() and qemuAgentFSThaw() are almost same. In addition, qemu guest agent provides status option for FS Freezer. So, could you change to following? VIR_ENUM_DECL(qemuAgentFSFreezeCtlMode); VIR_ENUM_IMPL(qemuAgentFSFreezeCtlMode, QEMU_AGENT_FSFREEZE_LAST, guest-fsfreeze-freeze, guest-fsfreeze-thaw, guest-fsfreeze-status); int qemuAgentFSFreezeCtl(qemuAGentPtr mon, qemuAgentFSFreezeCtlMode mode){ ... cmd = qemuAgentMakeCommand(qemuAgentFSFreezeCtlModeToString(mode), NULL); ... diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 93c2ae7..df59ef7 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -66,4 +66,7 @@ typedef enum { int qemuAgentShutdown(qemuAgentPtr mon, qemuAgentShutdownMode mode); +int qemuAgentFSFreeze(qemuAgentPtr mon); +int qemuAgentFSThaw(qemuAgentPtr mon); + #endif /* __QEMU_AGENT_H__ */ Similarly int qemuAgentFSFreezeCtl(qemuAgentPtr mon, qemuAgentFSFreezeCtlMode mode); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] qemu_agent: Create file system freeze and thaw functions
On 01/24/2012 04:17 PM, MATSUDA, Daiki wrote: I am happy that you provide the patches. These functions simply issue command to guest agent which should freeze or unfreeze all file systems within guest. --- But qemuAgentFSFreeze() and qemuAgentFSThaw() are almost same. In addition, qemu guest agent provides status option for FS Freezer. So, could you change to following? VIR_ENUM_DECL(qemuAgentFSFreezeCtlMode); VIR_ENUM_IMPL(qemuAgentFSFreezeCtlMode, QEMU_AGENT_FSFREEZE_LAST, guest-fsfreeze-freeze, guest-fsfreeze-thaw, guest-fsfreeze-status); That would make sense if we planned on exposing guest freeze and thaw directly to the user. But right now, we are only planning on using guest freeze and thaw internally as part of our higher-level snapshot create function, where we do freeze and thaw in a balanced pair, and where our command is blocking so that there is no need to call status (that is, if we exposed a libvirt command to query the status, you would only ever be able to call it when we are not in the middle of a snapshot, and thus the fs would always report that it is in a thawed state, unless someone has gone behind libvirt's back, at which point why are you using libvirt). Maybe you can convince me otherwise that we need to expose these three lower-level states to the user, rather than using them internally only for the implementation of the higher-level commands like snapshot-create, but unless we have a reason to expose this much detail, I don't think we need to make your proposed change. -- 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] Allow custom metadata in domain configuration XML
On 01/23/2012 07:26 PM, Zeeshan Ali (Khattak) wrote: From: Zeeshan Ali (Khattak) zeesha...@gnome.org Applications can now insert custom nodes and hierarchies into domain cofiguration XML. Although currently not enforced, application are s/cofiguration/configuration/ s/application are/applications are/ required to use their own namespaces on every custom node they insert. I also mentioned that we request only one element per namespace, in light of Dan's proposal for a new API that can get and set metadata on a per-namespace basis, so that applications don't have to filter all elements for the one they are interested in: https://www.redhat.com/archives/libvir-list/2012-January/msg00902.html --- docs/formatdomain.html.in | 18 + docs/schemas/domaincommon.rng | 26 + src/conf/domain_conf.c | 24 src/conf/domain_conf.h |3 ++ tests/domainsnapshotxml2xmlout/metadata.xml| 38 tests/domainsnapshotxml2xmltest.c |1 + tests/qemuxml2argvdata/qemuxml2argv-metadata.args |4 ++ tests/qemuxml2argvdata/qemuxml2argv-metadata.xml | 29 +++ .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml | 29 +++ tests/qemuxml2xmltest.c|2 + Thanks for taking in my suggestions. 'git send-email --subject-prefix=PATCHv2' makes it easier to state when you are sending a v2. I added you to AUTHORS; let me know if I need to update the spelling to match your preferred listing. +pre + ... + lt;metadatagt; +lt;app1:foo xmlns:app1=http://app1.org/app1/gt;..lt;/app1:foogt; +lt;app2:bar xmlns:app2=http://app1.org/app2/gt;..lt;/app2:bargt; + lt;/metadatagt; + .../pre Someday, we might try to list actual metadata (like danpb's suggestion of virtman:guest xmlns:virtman=http://virt-manager.org/guets/1.0;;); but I'm okay waiting for some implementation experience before listing something that actually appears in typical distro setups. + +dl + dtcodemetadata/code/dt + ddcodemetadata/code node could be used by applications to + store custom metadata in the form of XML nodes/trees. Applications + must use custom namespaces on their XML nodes/trees. + span class=sinceSince 0.9.9/span/dd since 0.9.10; and document the further restriction of only one element per namespace. I also think that this section belongs up next to description; but I'll move it as a followup patch. +++ b/docs/schemas/domaincommon.rng @@ -43,6 +43,9 @@ ref name=seclabel/ /optional optional + ref name=metadata/ +/optional +optional I'm moving this up next to description now. That is, both description and metadata are overall metadata about the domain, and thus belong next to one another. ref name='qemucmdline'/ /optional /interleave @@ -2942,6 +2945,29 @@ /element /define + define name=metadata +element name=metadata + zeroOrMore + ref name=customElement/ Indentation. +++ b/src/conf/domain_conf.c @@ -1500,6 +1500,9 @@ void virDomainDefFree(virDomainDefPtr def) if (def-namespaceData def-ns.free) (def-ns.free)(def-namespaceData); +if (def-metadata) +xmlFreeNode(def-metadata); Useless use of if before free, since xmlFreeNode is documented as a no-op on NULL. I'll update our list of function names to include xmlFreeNode(), so that 'make syntax-check' will prevent this in the future, as a followup patch. + VIR_FREE(def); } @@ -8072,6 +8075,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def-os.smbios_mode = VIR_DOMAIN_SMBIOS_NONE; /* not present */ } +/* Extract custom metadata */ +if ((node = virXPathNode(./metadata[1], ctxt)) != NULL) { +def-metadata = xmlCopyNode (node, 1); Style - libvirt doesn't use space before ( in function calls. @@ -11833,6 +11841,22 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; } +/* Custom metadata comes at the end */ +if (def-metadata) { +xmlBufferPtr xmlbuf; I'll move this earlier in a subsequent patch. + +xmlbuf = xmlBufferCreate(); +if (xmlNodeDump(xmlbuf, def-metadata-doc, def-metadata, 4, 1) 0) { I'm still not convinced you got this right. /me 2 hours later, after reading libxml2 docs Nope. The problem is that for indentation on output to work, we have to explicitly request ignoring extra whitespace on input, via xmlKeepBlanksDefault() (it defaults to 1, but must be 0 during the parse). Furthermore, the indentation generated by libxml defaults to 2 spaces per 1 level, which means that we need virBufferGetIndent / 2, to properly adjust things. +xmlBufferFree(xmlbuf); +
[libvirt] [PATCH] build: simplify xmlFreeNode usage
Noticed while reviewing the previous patch; thankfully, there are no violations. * cfg.mk (useless_free_options): Add xmlFreeNode. --- +if (def-metadata) +xmlFreeNode(def-metadata); Useless use of if before free, since xmlFreeNode is documented as a no-op on NULL. I'll update our list of function names to include xmlFreeNode(), so that 'make syntax-check' will prevent this in the future, as a followup patch. Pushing under the trivial rule. cfg.mk |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/cfg.mk b/cfg.mk index 20a085e..d853caf 100644 --- a/cfg.mk +++ b/cfg.mk @@ -171,6 +171,7 @@ useless_free_options = \ --name=xmlBufferFree \ --name=xmlFree \ --name=xmlFreeDoc\ + --name=xmlFreeNode \ --name=xmlXPathFreeContext \ --name=xmlXPathFreeObject -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] metadata: group metadata next to description
It's better to group all the metadata together. This is a cosmetic output change; since the RNG allows interleave, it doesn't matter where the user stuck it on input, and an XPath query will find it in the same location when parsing the output. * src/conf/domain_conf.c (virDomainDefFormatInternal): Output metadata earlier. * docs/formatdomain.html.in: Update documentation. * tests/domainsnapshotxml2xmlout/metadata.xml: Update test. * tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml: Likewise. --- As threatened here: I also think that this section belongs up next to description; but I'll move it as a followup patch. docs/formatdomain.html.in | 39 +++-- src/conf/domain_conf.c | 47 ++-- tests/domainsnapshotxml2xmlout/metadata.xml|8 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml |8 ++-- 4 files changed, 47 insertions(+), 55 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b025e8..dca9a81 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -33,6 +33,10 @@ lt;namegt;fv0lt;/namegt; lt;uuidgt;4dea22b31d52d8f32516782e98ab3fa0lt;/uuidgt; lt;descriptiongt;Some human readable descriptionlt;/descriptiongt; + lt;metadatagt; +lt;app1:foo xmlns:app1=http://app1.org/app1/gt;..lt;/app1:foogt; +lt;app2:bar xmlns:app2=http://app1.org/app2/gt;..lt;/app2:bargt; + lt;/metadatagt; .../pre dl @@ -56,9 +60,18 @@ dtcodedescription/code/dt ddThe content of the codedescription/code element provides a - human readable description of the virtual machine. This data is not - used by libvirt in any way, it can contain any information the user - wants. span class=sinceSince 0.7.2/span/dd +human readable description of the virtual machine. This data is not +used by libvirt in any way, it can contain any information the user +wants. span class=sinceSince 0.7.2/span/dd + + dtcodemetadata/code/dt + ddThe codemetadata/code node can be used by applications +to store custom metadata in the form of XML +nodes/trees. Applications must use custom namespaces on their +XML nodes/trees, with only one top-level element per namespace +(if the application needs structure, they should have +sub-elements to their namespace +element). span class=sinceSince 0.9.10/span/dd /dl h3a name=elementsOSOperating system booting/a/h3 @@ -3556,26 +3569,6 @@ qemu-kvm -net nic,model=? /dev/null sub-element codelabel/code are supported. /p -h3a name=customMetadataCustom metadata/a/h3 - -pre - ... - lt;metadatagt; -lt;app1:foo xmlns:app1=http://app1.org/app1/gt;..lt;/app1:foogt; -lt;app2:bar xmlns:app2=http://app1.org/app2/gt;..lt;/app2:bargt; - lt;/metadatagt; - .../pre - -dl - dtcodemetadata/code/dt - ddThe codemetadata/code node can be used by applications to - store custom metadata in the form of XML nodes/trees. Applications - must use custom namespaces on their XML nodes/trees, with only - one top-level element per namespace (if the application needs - structure, they should have sub-elements to their namespace - element). span class=sinceSince 0.9.10/span/dd -/dl - h2a name=examplesExample configs/a/h2 p diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f2c8d02..e872396 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11431,6 +11431,29 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferEscapeString(buf, description%s/description\n, def-description); +if (def-metadata) { +xmlBufferPtr xmlbuf; +int oldIndentTreeOutput = xmlIndentTreeOutput; + +/* Indentation on output requires that we previously set + * xmlKeepBlanksDefault to 0 when parsing; also, libxml does 2 + * spaces per level of indentation of intermediate elements, + * but no leading indentation before the starting element. + * Thankfully, libxml maps what looks like globals into + * thread-local uses, so we are thread-safe. */ +xmlIndentTreeOutput = 1; +xmlbuf = xmlBufferCreate(); +if (xmlNodeDump(xmlbuf, def-metadata-doc, def-metadata, +virBufferGetIndent(buf, false) / 2 + 1, 1) 0) { +xmlBufferFree(xmlbuf); +xmlIndentTreeOutput = oldIndentTreeOutput; +goto cleanup; +} +virBufferAsprintf(buf, %s\n, (char *) xmlBufferContent(xmlbuf)); +xmlBufferFree(xmlbuf); +xmlIndentTreeOutput = oldIndentTreeOutput; +} + virBufferAsprintf(buf, memory%lu/memory\n, def-mem.max_balloon); virBufferAsprintf(buf, currentMemory%lu/currentMemory\n, def-mem.cur_balloon); @@ -11844,30 +11867,6 @@
Re: [libvirt] [PATCH] metadata: group metadata next to description
On Wed, Jan 25, 2012 at 2:28 AM, Eric Blake ebl...@redhat.com wrote: It's better to group all the metadata together. This is a cosmetic output change; since the RNG allows interleave, it doesn't matter where the user stuck it on input, and an XPath query will find it in the same location when parsing the output. Looks right. ACK! -- Regards, Zeeshan Ali (Khattak) FSF member#5124 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] metadata: group metadata next to description
On 01/24/2012 05:49 PM, Zeeshan Ali (Khattak) wrote: On Wed, Jan 25, 2012 at 2:28 AM, Eric Blake ebl...@redhat.com wrote: It's better to group all the metadata together. This is a cosmetic output change; since the RNG allows interleave, it doesn't matter where the user stuck it on input, and an XPath query will find it in the same location when parsing the output. Looks right. ACK! Thanks; pushed. -- 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] docs: fix a few small typos in formatdomain.html.in
--- Pushed under the trivial rule. docs/formatdomain.html.in | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dca9a81..dfb010d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2275,8 +2275,8 @@ pre ... lt;devicesgt; -lt;mac address='52:54:00:6d:90:01'gt; lt;interface type='mcast'gt; + lt;mac address='52:54:00:6d:90:01'gt; lt;source address='230.0.0.1' port='5558'/gt; lt;/interfacegt; lt;/devicesgt; @@ -2297,13 +2297,13 @@ ... lt;devicesgt; lt;interface type='server'gt; -lt;mac address='52:54:00:22:c9:42'gt; + lt;mac address='52:54:00:22:c9:42'gt; lt;source address='192.168.0.1' port='5558'/gt; lt;/interfacegt; ... lt;interface type='client'gt; -lt;mac address='52:54:00:8b:c9:51'gt; -lt;source address='192.168.0.1' port='5558'/gt; + lt;mac address='52:54:00:8b:c9:51'gt; + lt;source address='192.168.0.1' port='5558'/gt; lt;/interfacegt; lt;/devicesgt; .../pre @@ -2470,7 +2470,7 @@ qemu-kvm -net nic,model=? /dev/null .../pre p - For hypervisors which support this, you can set exact NIC which should + For hypervisors which support this, you can set a specific NIC to be used for network boot. The codeorder/code attribute determines the order in which devices will be tried during boot sequence. The per-device codeboot/code elements cannot be used together with -- 1.7.7.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH][RFC] adding a title to the domain description informations
On Tue, Jan 24, 2012 at 08:06:31AM -0700, Eric Blake wrote: On 01/24/2012 07:18 AM, Daniel Veillard wrote: The idea is that currently we have only the domain name usable as a description for the domain. It is not really a good human readable identifier, as the kind of string allowed is limited (no space for example). The idea would then be to extend the existing description field in the domain XML to keep 40 or less character string to describe a domain and provide that information later for example in an extended virsh list command or for other interfaces. While the idea is simple, see attached patch for this, it becomes more complex when one tries to make accessors to set/get that title for a domain, since it's mutable and possibly could be coming from the hypervisor itself (is there anything like this in VMWare or VirtualBox?) it should to be implemented down at the driver level. Is that worth the effort ? If we go that route should we do this for other objects (network, storage, etc ...) too in the end ? here is a basic patch for just the XML side to give an idea, but adding APIs is far more work. Opinions ? Looks like this is more or less identical to Peter's proposal to add a title element: https://www.redhat.com/archives/libvir-list/2012-January/msg00710.html so I'm all for the idea (not sure whether your patch or Peter's is better for the idea). As far as I know, the title is _not_ coming from the hypervisor itself, nor is it guest-visible; it is merely metadata from the host's perspective. Okay, let's go with Peter's then, the only difference is that he is using an element to store the title instead of an attribute on the description element. That may be simpler in the end, so I'm fine dropping my patch :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list