Re: [libvirt] [Dnsmasq-discuss] Chaining instances?
On 02/25/2013 02:05 AM, Laine Stump wrote: This discussion should really be taking place on libvir-list - I'm Cc'ing it there. Bah. I'm not sure what happened, but I ended up not actually Cc'ing... On 02/24/2013 04:11 PM, TJ wrote: On 24/02/13 19:19, Laine Stump wrote: On 02/24/2013 05:09 AM, TJ wrote: I wondered if maybe configuring the libvirtd dnsmasq instances to be dhcp proxies for the LAN dnsmasq, and use multiple dhcp-range's with tags might do it? My brain is a bit fried right now having had to fix several bugs in vmbuilder and libvirt, plus add new functionality to libvirtd to allow its dnsmasq instance to read a conf-file and use a separate log-facility, What you're talking about doing sounds *very* useful to have supported directly in libvirt, but you may want to contact libvirt's upstream developers (at libvir-list@redhat.com, or on irc.oftc.net in #virt) prior to expending a lot of effort on libvirt code - in general a problem may already be solved in a different manner, or somebody else may already be working on it. Failing that, there may have already been considerable debate on a particular subject, and a path to a solution generally agreed on, but with nobody yet working on the code. It turns out that what I need(ed) to do was completely *disable* libvirt's use of dnsmasq and instead use Simon's related dhcp-helper program which acts as a full dhcp-relay using, f.e: /usr/sbin/dhcp-helper -u libvirt-dnsmasq -i virbr1 -b eth0 This forwards all requests to the primary dnsmasq DHCP server on the LAN which matches against the appropriate dhcp-range: # physical network leases dhcp-range=set:phys,10.254.251.50,10.254.251.199,255.255.255.0,1440m # subnet for VMs on server1 dhcp-range=set:vmlan1,10.254.1.100,10.254.1.199,255.255.255.0,1440m # default route (gateway) dhcp-option=tag:phys,option:router,10.254.251.1 dhcp-option=tag:vmlan1,option:router,10.254.1.1 # DNS server dhcp-option=6,10.254.251.1 To that end this evening I added two additional options to libvirt: dnsmasq enabled='true'/ dhcphelper enabled='false'/ libvirt's XML should remain implementation-agnostic. We don't want to have an option to disable the specific implementation of dns+dhcp called dnsmasq; rather we want to be able to enable/disable a network's dhcp server and/or dns server. (The reason for this is that we want to leave the door open for someone to implement a different backend using a different dhcp server and/or dns server and be able to use the same configuration.) We have actually previously discussed disabling dns and/or dhcp (see http://www.redhat.com/archives/libvir-list/2012-November/msg00861.html). It is already possible to completely disable dhcp - simply don't include a dhcp element in the network definition. As for dns, from the very beginning dns has been *always* enabled on all networks, so even though there is a dns element, simply having not having one is not a valid way to say no dns server (as it would cause backward compatibility problems with existing installations). Instead, the conclusion of the above-mentioned thread was that the proper way to handle this would be to add an enable element to the existing dns element, which would default to yes. To disable libvirt-supplied dns services for a subnet, you would just put: dns enable='no'/ in the network definition. In the case that dns had enable='no' AND there was no dhcp element, dnsmasq simply wouldn't be started at all. That hasn't been implemented yet, but shouldn't be too complex to do. As for dhcp-helper, aside from the fact that again you've created an option that is specific to a particular implementation of what is needed, that command isn't universally available anyway (it's not in any package for Fedora/RHEL, for example), and I'm not sure that it's really necessary to have it started up by libvirt anyway - can it detect newly created/upped interfaces as dnsmasq can? If so, it could be started up by regular host system config even before libvirt was started. These are the default values when the options are *not* defined. They allow the admin to disable dnsmasq entirely: dnsmasq enabled='false'/ and to enable dhcp-helper: dhcphelper enabled='true'/ Using two new functions: int networkBuildDhcphelperArgv(...) int networkBuildDhcpHelperCommandLine(...) the existing function: int networkStartDhcpDaemon(...) is able to launch either or both of dnsmasq and dhcp-helper with the correct options. dhcp-helper's -i option is filled from network-def-bridge and its -b option is taken from the first forward device declared in a forward dev='?'/ or interface dev='?'/. If no forward device is declared it throws a VIR_ERR_INVALID_INTERFACE error with appropriate explanatory text. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support
On 02/22/13 19:20, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: This patch documents XML elements used for (basic) support of virtual RNG devices. In the devices section in the domain XML users may specify: For the default 'random' backend: devices rng model='virtio' backend model='random'/dev/urandom/backend /rng /devices For the slightly more advanced EGD backend: devices rng model='virtio' backend model='egd' type='udp' !-- this is a definition of a character device -- source mode='bind' service='1234'/ source mode='connect' host='1.2.3.4' service='1234'/ !-- or other valid character device configuration -- You don't really allow two source; maybe a better layout would be a strategic comment, such as: backend model='egd' type='udp' !-- this is a definition of a character device -- source mode='bind' service='1234'/ !-- or other valid character device configuration, such as source mode='connect' host='1.2.3.4' service='1234'/ -- /backend /rng /devices For the planned random daemon/pool: devices rng model='virtio' backend model='pool' pool='poolname'class/backend Missing /rng /devices to enable the RNG device for guests. --- Notes: Version 2: - ACKed, no change, unfortunately doesn't make sense to push alone Still some nits to fix before pushing: +pre + ... + lt;devicesgt; +lt;rng model='virtio'gt; + lt;backend model='random'gt;/dev/randomlt;/backendgt; + lt;!-- OR --gt; + lt;backend model='egd' type='udp'gt; +lt;source mode='bind' service='1234'gt; +lt;source mode='connect' host='1.2.3.4' service='1234'gt; + lt;/backendgt; +lt;/rnggt; + lt;/devicesgt; + ... Do we really want two source in a single backend in the example, or would it be easier to show multiple rng devices, one for each type of backend? That actually is valid for the character device backends. The UDP backend has to use two separate sources for bi-directional communication. The definition of that source type is declared as a type in our RNG schema an I merely reused that. + dd +p + The codebackend/code element specifies the source of entropy + to be used for the doimain. The source model is configured using the s/doimain/domain/ + codemodel/code attribute. Supported source models are: +/p +ul + li'random' mdash; /dev/random (default) or similar device as source/li + li'egd' mdash; a EGD protocol backend. /li not consistent on whether your li end with '.' Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/8] conf: Add support for RNG device configuration in XML
On 02/23/13 00:33, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: This patch adds basic configuration support for the RNG device suporting s/suporting/supporting/ the virtio model with the random and egd backend types as described in the schema in the previous patch. --- Notes: Version 2: - fix a ton of memory leaks (I assumed that virXMLGetProp returns static strings) - Add new device type to even more places - Fix error message cp error - Fix memleak in RNGDef free func ... @@ -10601,6 +10734,22 @@ virDomainDefParseXML(virCapsPtr caps, } } +/* Parse the RNG device */ +if ((n = virXPathNodeSet(./devices/rng, ctxt, nodes)) 0) +goto error; + +if (n 1) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(only a single RNG device is supported)); Is this an inherent limit of qemu? For that matter, is it an inherent limit, and no hypervisor can ever support more than one? In the bare metal case, can't you plug in multiple rng hardware dongles? AFAIK qemu is able to support multiple RNG devices but I didn't find that particularly useful. I will follow up with a patch that will add support for more than one RNG device, although I don't think it will be used much. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] network: add force attribute for dhcp options
On 25.02.2013 05:37, Laine Stump wrote: The firs patch just fixes unexpected behavior in virNetworkDHCPOptionDefParseXML found when testing the second patch. The second patch adds the force attribute I mentioned in the review of the patch that added support for specifying dhcp options in network config. While documenting this new attribute, I noticed that I'd neglected to see that original patch didn't include documentation, so I added full documentation for option in order to have a place to add the documentation for force='yes'. I'm also working on a way to allow specification of option names rather than numbers, but didn't know if I could get it in before the freeze, so I'm sending this patch now. Laine Stump (2): conf: use VIR_EXPAND instead of VIR_REALLOC in virNetworkDHCPDefParseXML network: add force attribute for dhcp options docs/formatnetwork.html.in | 21 src/conf/network_conf.c| 32 +- src/conf/network_conf.h| 3 ++- src/network/bridge_driver.c| 3 ++- tests/networkxml2confdata/nat-network.conf | 2 +- tests/networkxml2confdata/nat-network.xml | 2 +- 6 files changed, 50 insertions(+), 13 deletions(-) ACK to both patches. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Dnsmasq-discuss] Chaining instances?
This discussion should really be taking place on libvir-list - I'm Cc'ing it there. On 02/24/2013 04:11 PM, TJ wrote: On 24/02/13 19:19, Laine Stump wrote: On 02/24/2013 05:09 AM, TJ wrote: I wondered if maybe configuring the libvirtd dnsmasq instances to be dhcp proxies for the LAN dnsmasq, and use multiple dhcp-range's with tags might do it? My brain is a bit fried right now having had to fix several bugs in vmbuilder and libvirt, plus add new functionality to libvirtd to allow its dnsmasq instance to read a conf-file and use a separate log-facility, What you're talking about doing sounds *very* useful to have supported directly in libvirt, but you may want to contact libvirt's upstream developers (at libvir-list@redhat.com, or on irc.oftc.net in #virt) prior to expending a lot of effort on libvirt code - in general a problem may already be solved in a different manner, or somebody else may already be working on it. Failing that, there may have already been considerable debate on a particular subject, and a path to a solution generally agreed on, but with nobody yet working on the code. It turns out that what I need(ed) to do was completely *disable* libvirt's use of dnsmasq and instead use Simon's related dhcp-helper program which acts as a full dhcp-relay using, f.e: /usr/sbin/dhcp-helper -u libvirt-dnsmasq -i virbr1 -b eth0 This forwards all requests to the primary dnsmasq DHCP server on the LAN which matches against the appropriate dhcp-range: # physical network leases dhcp-range=set:phys,10.254.251.50,10.254.251.199,255.255.255.0,1440m # subnet for VMs on server1 dhcp-range=set:vmlan1,10.254.1.100,10.254.1.199,255.255.255.0,1440m # default route (gateway) dhcp-option=tag:phys,option:router,10.254.251.1 dhcp-option=tag:vmlan1,option:router,10.254.1.1 # DNS server dhcp-option=6,10.254.251.1 To that end this evening I added two additional options to libvirt: dnsmasq enabled='true'/ dhcphelper enabled='false'/ libvirt's XML should remain implementation-agnostic. We don't want to have an option to disable the specific implementation of dns+dhcp called dnsmasq; rather we want to be able to enable/disable a network's dhcp server and/or dns server. (The reason for this is that we want to leave the door open for someone to implement a different backend using a different dhcp server and/or dns server and be able to use the same configuration.) We have actually previously discussed disabling dns and/or dhcp (see http://www.redhat.com/archives/libvir-list/2012-November/msg00861.html). It is already possible to completely disable dhcp - simply don't include a dhcp element in the network definition. As for dns, from the very beginning dns has been *always* enabled on all networks, so even though there is a dns element, simply having not having one is not a valid way to say no dns server (as it would cause backward compatibility problems with existing installations). Instead, the conclusion of the above-mentioned thread was that the proper way to handle this would be to add an enable element to the existing dns element, which would default to yes. To disable libvirt-supplied dns services for a subnet, you would just put: dns enable='no'/ in the network definition. In the case that dns had enable='no' AND there was no dhcp element, dnsmasq simply wouldn't be started at all. That hasn't been implemented yet, but shouldn't be too complex to do. As for dhcp-helper, aside from the fact that again you've created an option that is specific to a particular implementation of what is needed, that command isn't universally available anyway (it's not in any package for Fedora/RHEL, for example), and I'm not sure that it's really necessary to have it started up by libvirt anyway - can it detect newly created/upped interfaces as dnsmasq can? If so, it could be started up by regular host system config even before libvirt was started. These are the default values when the options are *not* defined. They allow the admin to disable dnsmasq entirely: dnsmasq enabled='false'/ and to enable dhcp-helper: dhcphelper enabled='true'/ Using two new functions: int networkBuildDhcphelperArgv(...) int networkBuildDhcpHelperCommandLine(...) the existing function: int networkStartDhcpDaemon(...) is able to launch either or both of dnsmasq and dhcp-helper with the correct options. dhcp-helper's -i option is filled from network-def-bridge and its -b option is taken from the first forward device declared in a forward dev='?'/ or interface dev='?'/. If no forward device is declared it throws a VIR_ERR_INVALID_INTERFACE error with appropriate explanatory text. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] tests: avoid segfault if json monitor not present
On 23.02.2013 00:09, Eric Blake wrote: On a machine without json headers, I was seeing random segfaults from qemumonitorjsontest (about 90% of the runs on my particular machine). The segfault was inside virClassIsDerivedFrom, which points to a case of a race leading to unreferencing a stale pointer to an object that had already been freed. I also noticed that if I got the segfault, I was seeing messages such as: 2013-02-22 16:12:37.504+: 19833: error : virNetSocketWriteWire:1361 : Cannot write data: Bad file descriptor which is also evidence of deferencing a stale pointer. I traced it to a race where qemuMonitorTestIO could execute late, after the main thread had already called qemuMonitorTestFree and called virNetSocketClose(test-client) but not clearing it out to NULL. Sure enough, after test-client has been closed, fd is -1, which causes an attempt to write to the socket to fail, which in turn triggers the error code of qemuMonitorTestIO that tries to re-close test-client. * tests/qemumonitortestutils.c (qemuMonitorTestIO): Don't attempt to free client again if test already quit. --- tests/qemumonitortestutils.c | 4 1 file changed, 4 insertions(+) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 1ed42ce..979623a 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -214,6 +214,10 @@ static void qemuMonitorTestIO(virNetSocketPtr sock, bool err = false; virMutexLock(test-lock); +if (test-quit) { +virMutexUnlock(test-lock); +return; +} if (events VIR_EVENT_HANDLE_WRITABLE) { ssize_t ret; if ((ret = virNetSocketWrite(sock, ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: minor monitor lock cleanups
On 23.02.2013 00:09, Eric Blake wrote: If virCondInit fails (okay, so that's unlikely), then we end up attempting a virObjectUnlock() on the cleanup path, even though we don't hold a lock. This is not guaranteed to be safe. While at it, I noticed a couple places where we were referencing mon-fd outside locks. * src/qemu/qemu_monitor.c (qemuMonitorOpenInternal): Minimize lock duration. mon-watch doesn't need clean up on error. (qemuMonitorGetBlockExtent, qemuMonitorBlockResize): Don't dereference fd outside of lock. --- src/qemu/qemu_monitor.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7f4a7a0..40eea75 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -717,7 +717,6 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, if (json) mon-wait_greeting = 1; mon-cb = cb; -virObjectLock(mon); if (virSetCloseExec(mon-fd) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -731,6 +730,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, } +virObjectLock(mon); virObjectRef(mon); if ((mon-watch = virEventAddHandle(mon-fd, VIR_EVENT_HANDLE_HANGUP | @@ -740,6 +740,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, mon, virObjectFreeCallback)) 0) { virObjectUnref(mon); +virObjectUnlock(mon); Heh, it took me while to realize this is good actually. I thought it should be vice versa to prevent Unlock() being called on just freed memory. But then I realized after the Unref() mon refcount == 1. virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(unable to register monitor events)); goto cleanup; @@ -759,11 +760,8 @@ cleanup: * so kill the callbacks now. */ mon-cb = NULL; -virObjectUnlock(mon); /* The caller owns 'fd' on failure */ mon-fd = -1; -if (mon-watch) -virEventRemoveHandle(mon-watch); qemuMonitorClose(mon); return NULL; } @@ -1508,8 +1506,7 @@ int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, unsigned long long *extent) { int ret; -VIR_DEBUG(mon=%p, fd=%d, dev_name=%p, - mon, mon-fd, dev_name); +VIR_DEBUG(mon=%p, dev_name=%p, mon, dev_name); if (mon-json) ret = qemuMonitorJSONGetBlockExtent(mon, dev_name, extent); @@ -1524,8 +1521,7 @@ int qemuMonitorBlockResize(qemuMonitorPtr mon, unsigned long long size) { int ret; -VIR_DEBUG(mon=%p, fd=%d, devname=%p size=%llu, - mon, mon-fd, device, size); +VIR_DEBUG(mon=%p, devname=%p size=%llu, mon, device, size); if (mon-json) ret = qemuMonitorJSONBlockResize(mon, device, size); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] qemu: don't override earlier json error
On 23.02.2013 00:09, Eric Blake wrote: I built without json support, and noticed a strange failure message in qemumonitorjsontest: 2013-02-22 16:12:37.503+: 19812: error : virJSONValueToString:1119 : internal error No JSON parser implementation is available 2013-02-22 16:12:37.503+: 19812: error : qemuMonitorJSONCommandWithFd:253 : out of memory While a later patch will fix the test to skip when json is not present, this patch avoids overriding the more useful error message from virJSONValueToString returning NULL. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONCommandWithFd): Don't override message. (qemuMonitorJSONCheckError): Don't print NULL. * src/qemu/qemu_agent.c (qemuAgentCommand): Don't override message. (qemuAgentCheckError): Don't print NULL. (qemuAgentArbitraryCommand): Properly fail on OOM. --- src/qemu/qemu_agent.c| 13 ++--- src/qemu/qemu_monitor_json.c | 8 +++- 2 files changed, 9 insertions(+), 12 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] tests: don't test json when not compiled in
On 23.02.2013 00:09, Eric Blake wrote: Now that the segfault is solved, we can skip instead of fail the test when yajl is not present. * tests/qemumonitorjsontest.c (mymain): Skip if no yajl. --- tests/qemumonitorjsontest.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 55032d6..e2f8cb1 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011-2012 Red Hat, Inc. + * Copyright (C) 2011-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -444,2 +443,2 @@ mymain(void) int ret = 0; virCapsPtr caps; +#if !WITH_YAJL +fprintf(stderr, libvirt not compiled with yajl, skipping); +return EXIT_AM_SKIP; +#endif + if (virThreadInitialize() 0) exit(EXIT_FAILURE); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] tests: uniformly report test failures
On 23.02.2013 00:09, Eric Blake wrote: testutils.c likes to print summaries after a test completes, including if it failed. But if the test outright exit()s, this summary is skipped. Enforce that we return instead of exit. * cfg.mk (sc_prohibit_exit_tests): New syntax check. * tests/commandhelper.c (main): Fix offenders. * tests/qemumonitorjsontest.c (mymain): Likewise. * tests/seclabeltest.c (main): Likewise. * tests/securityselinuxlabeltest.c (mymain): Likewise. * tests/securityselinuxtest.c (mymain): Likewise. * tests/testutils.h (VIRT_TEST_MAIN_PRELOAD): Likewise. * tests/testutils.c (virtTestMain): Likewise. (virtTestCaptureProgramOutput): Use symbolic name. --- cfg.mk | 7 +++ tests/commandhelper.c| 7 +++ tests/qemumonitorjsontest.c | 8 +++- tests/seclabeltest.c | 8 tests/securityselinuxlabeltest.c | 6 +++--- tests/securityselinuxtest.c | 6 +++--- tests/testutils.c| 8 tests/testutils.h| 4 ++-- 8 files changed, 29 insertions(+), 25 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 7/8] tests: Add tests for virtio-rng device handling
On 02/23/13 01:29, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: Adds XML parsing and qemu commandline tests for the VirtIO RNG device support. --- Notes: Version 2: - ACKed .../qemuxml2argv-virtio-rng-egd.args | 1 + .../qemuxml2argv-virtio-rng-egd.xml| 26 ++ .../qemuxml2argv-virtio-rng-random.args| 1 + .../qemuxml2argv-virtio-rng-random.xml | 23 +++ tests/qemuxml2argvtest.c | 5 + tests/qemuxml2xmltest.c| 3 +++ 6 files changed, 59 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-egd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-egd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml Is it worth testing that a filename containing an XML-special character is properly escaped? Other than that, this one is still good to go. +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml @@ -0,0 +1,23 @@ +rng model='virtio' + backend model='random'/test/phile/backend That is, should this use something like /test/lt;phile as the XML encoded file name? Uh, I'm not following you on this one. You mean that if the user specifies some characters that are invalid from the perspective of XML as a source path? Anyways, I fixed the issues you pointed out in 1-6 and provided explanation for the other stuff. I'm pushing patches 1-7 (the test suite can be improved at any time) now and will follow up later with a improved version of 8 as well as with a patch that will allow multiple RNG devices. That should be better to review as slicing apart the existing patches. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 resend 1/4] add domain NIC model enum macro
--- src/conf/domain_conf.c | 31 +++ src/conf/domain_conf.h | 23 +++ 2 files changed, 54 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0c75838..90a6359 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -700,6 +700,37 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, static, auto); +/* For NIC model macro, a comment marks the start of a model + * group which ends with the model just before next comment + * or extends to the end of list. + */ +VIR_ENUM_IMPL(virDomainNICModel, + VIR_DOMAIN_NIC_MODEL_LAST, + default, + spapr-vlan, /* qemu */ + + virtio, /* qemu and vbox */ + + ne2k_isa, /* qemu and Xen */ + ne2k_pci, + pcnet, + rtl8139, + + e1000, /* qemu, Xen and VMX */ + + netfront, /* Xen(hvm) and libxl */ + + vlance, /* VMX */ + vmxnet, + vmxnet2, + vmxnet3, + + Am79C970A, /* vbox */ + Am79C973, + 82540EM, + 82545EM, + 82543GC); + #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4ffa4aa..0a06e11 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -782,6 +782,28 @@ struct _virDomainFSDef { unsigned long long space_soft_limit; /* in bytes */ }; +enum virDomainNICModel { +VIR_DOMAIN_NIC_MODEL_DEFAULT = 0, +VIR_DOMAIN_NIC_MODEL_SPAPR_VLAN, +VIR_DOMAIN_NIC_MODEL_VIRTIO, +VIR_DOMAIN_NIC_MODEL_NE2K_ISA, +VIR_DOMAIN_NIC_MODEL_NE2K_PCI, +VIR_DOMAIN_NIC_MODEL_PCNET, +VIR_DOMAIN_NIC_MODEL_RTL8139, +VIR_DOMAIN_NIC_MODEL_E1000, +VIR_DOMAIN_NIC_MODEL_NETFRONT, +VIR_DOMAIN_NIC_MODEL_VLANCE, +VIR_DOMAIN_NIC_MODEL_VMXNET, +VIR_DOMAIN_NIC_MODEL_VMXNET2, +VIR_DOMAIN_NIC_MODEL_VMXNET3, +VIR_DOMAIN_NIC_MODEL_AM79C970A, +VIR_DOMAIN_NIC_MODEL_AM79C973, +VIR_DOMAIN_NIC_MODEL_82540EM, +VIR_DOMAIN_NIC_MODEL_82545EM, +VIR_DOMAIN_NIC_MODEL_82543GC, + +VIR_DOMAIN_NIC_MODEL_LAST +}; /* 5 different types of networking config */ enum virDomainNetType { @@ -2324,6 +2346,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceClipboardCopypaste) VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode) VIR_ENUM_DECL(virDomainNumatuneMemMode) VIR_ENUM_DECL(virDomainNumatuneMemPlacementMode) +VIR_ENUM_DECL(virDomainNICModel) VIR_ENUM_DECL(virDomainHyperv) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) -- 1.7.11.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 resend 0/4]use VIR_ENUM_DECL for domain NIC model and add usb-net
v1 to v2 *removed i8* NIC models support for QEMU/KVM *updated docs *rebased This patchset uses VIR_ENUM_DECL and VIR_ENUM_IMPL macros for NIC models and changes the related codes to use them. All of NIC models supported by various hypervisors comprise of the enum, so the problem is that we need to do further checking in hypervisor-specific implementation. Fortunately, VMX and Vbox did this checking very well already. In 4/4, a similar job is performed for QEMU/KVM. And, adds usb-net support. The vendorId and productID of is 0525:a4a2. The corresponding item in usb-ids is as follows: 0525 Netchip Technology, Inc. a4a2 Linux-USB Ethernet/RNDIS Gadget In my opinion, using usb-net directly is good enough, so I keep it. Any other idea is welcome. Libvirt XML sample: devices interface type='user' mac address='52:54:00:32:6a:91'/ model type='usb-net'/ alias name='net1'/ address type='usb' bus='0' port='1'/ /interface /devices qemu commandline: qemu ${other_vm_args} -netdev user,id=hostnet1 \ -device usb-net,netdev=hostnet1,id=net1,\ mac=52:54:00:32:6a:91,bus=usb.0,port=1 Guannan Ren(4) [PATCH v2 resend 1/4] add domain NIC model enum macro [PATCH v2 resend 2/4] net: use virDomainNICModelType{From|To}String functions [PATCH v2 resend 3/4] qemu: add NIC model checking for qemu hypervisor [PATCH v2 resend 4/4] qemu: add usb-net support docs/formatdomain.html.in| 29 +++-- src/conf/domain_conf.c | 78 ++ src/conf/domain_conf.h | 32 +++- src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 5 +++-- src/parallels/parallels_driver.c | 2 +- src/qemu/qemu_command.c | 41 +++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 10 ++ src/qemu/qemu_process.c | 14 +++--- src/vbox/vbox_tmpl.c | 57 +++-- src/vmx/vmx.c| 31 --- src/xenxs/xen_sxpr.c | 21 +++-- src/xenxs/xen_xm.c | 21 +++-- 14 files changed, 208 insertions(+), 137 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 resend 3/4] qemu: add NIC model checking for qemu hypervisor
As the enum virDomainNICModel is a big collection of NIC models for all of hypervisors, for qemu/kvm, only some of them are supported, so the patch tries to add a checking for NIC model that is qemu specific. The way of doing this is the same as VMX and Vbox do which have codes to validate the hypervisor-specific NIC model in their implementation rather than domain XML parsing code. --- src/conf/domain_conf.h | 6 ++ src/qemu/qemu_command.c | 7 +++ 2 files changed, 13 insertions(+) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6425290..687032b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -791,6 +791,12 @@ enum virDomainNICModel { VIR_DOMAIN_NIC_MODEL_PCNET, VIR_DOMAIN_NIC_MODEL_RTL8139, VIR_DOMAIN_NIC_MODEL_E1000, + +/* Add new NIC model for qemu above this. + */ +VIR_DOMAIN_NIC_MODEL_FOR_QEMU_END = +VIR_DOMAIN_NIC_MODEL_E1000, + VIR_DOMAIN_NIC_MODEL_NETFRONT, VIR_DOMAIN_NIC_MODEL_VLANCE, VIR_DOMAIN_NIC_MODEL_VMXNET, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5cb013c..3a98566 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6032,6 +6032,13 @@ qemuBuildCommandLine(virConnectPtr conn, else vlan = i; +if (net-model 0 || net-model VIR_DOMAIN_NIC_MODEL_FOR_QEMU_END) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(NIC model '%s' is unsupported for QEMU), + virDomainNICModelTypeToString(net-model)); +goto error; +} + /* If appropriate, grab a physical device from the configured * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. -- 1.7.11.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 resend 2/4] net: use virDomainNICModelType{From|To}String functions
--- src/conf/domain_conf.c | 32 -- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 ++ src/libxl/libxl_conf.c | 5 ++-- src/parallels/parallels_driver.c | 2 +- src/qemu/qemu_command.c | 26 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 10 --- src/qemu/qemu_process.c | 14 +- src/vbox/vbox_tmpl.c | 57 +++- src/vmx/vmx.c| 31 +++--- src/xenxs/xen_sxpr.c | 21 --- src/xenxs/xen_xm.c | 21 --- 13 files changed, 110 insertions(+), 115 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 90a6359..96203e2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1150,8 +1150,6 @@ void virDomainNetDefFree(virDomainNetDefPtr def) if (!def) return; -VIR_FREE(def-model); - switch (def-type) { case VIR_DOMAIN_NET_TYPE_ETHERNET: VIR_FREE(def-data.ethernet.dev); @@ -5220,9 +5218,6 @@ error: return ret; } -#define NET_MODEL_CHARS \ -abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_- - /* Parse the XML definition for a network interface * @param node XML nodeset to parse for net definition * @return 0 on success, -1 on failure @@ -5592,23 +5587,17 @@ virDomainNetDefParseXML(virCapsPtr caps, ifname = NULL; } -/* NIC model (see -net nic,model=?). We only check that it looks - * reasonable, not that it is a supported NIC type. FWIW kvm - * supports these types as of April 2008: - * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio - * QEMU PPC64 supports spapr-vlan - */ if (model != NULL) { -if (strspn(model, NET_MODEL_CHARS) strlen(model)) { -virReportError(VIR_ERR_INVALID_ARG, %s, - _(Model name contains invalid characters)); +int m; +if ((m = virDomainNICModelTypeFromString(model)) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Unknown NIC model has been specified)); goto error; } -def-model = model; -model = NULL; +def-model = m; } -if (def-model STREQ(def-model, virtio)) { +if (def-model == VIR_DOMAIN_NIC_MODEL_VIRTIO) { if (backend != NULL) { int name; if ((name = virDomainNetBackendTypeFromString(backend)) 0 || @@ -11254,10 +11243,11 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } -if (STRNEQ_NULLABLE(src-model, dst-model)) { +if (src-model != dst-model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(Target network card model %s does not match source %s), - NULLSTR(dst-model), NULLSTR(src-model)); + virDomainNICModelTypeToString(dst-model), + virDomainNICModelTypeToString(src-model)); return false; } @@ -13175,8 +13165,8 @@ virDomainNetDefFormat(virBufferPtr buf, } if (def-model) { virBufferEscapeString(buf, model type='%s'/\n, - def-model); -if (STREQ(def-model, virtio) + virDomainNICModelTypeToString(def-model)); +if ((def-model == VIR_DOMAIN_NIC_MODEL_VIRTIO) (def-driver.virtio.name || def-driver.virtio.txmode)) { virBufferAddLit(buf, driver); if (def-driver.virtio.name) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0a06e11..6425290 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -879,7 +879,7 @@ struct _virDomainActualNetDef { struct _virDomainNetDef { enum virDomainNetType type; virMacAddr mac; -char *model; +int model; union { struct { enum virDomainNetBackendType name; /* which driver backend to use */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f399871..499add9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -250,6 +250,8 @@ virDomainNetGetActualVlan; virDomainNetInsert; virDomainNetRemove; virDomainNetTypeToString; +virDomainNICModelTypeFromString; +virDomainNICModelTypeToString; virDomainNostateReasonTypeFromString; virDomainNostateReasonTypeToString; virDomainNumatuneMemModeTypeFromString; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4ce5dec..c963834 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -587,8 +587,9 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic) virMacAddrGetRaw(l_nic-mac, x_nic-mac); -if (l_nic-model !STREQ(l_nic-model, netfront)) { -if ((x_nic-model = strdup(l_nic-model)) == NULL) { +if (l_nic-model
[libvirt] [PATCH v2 resend 4/4] qemu: add usb-net support
For USB device, the address is optional, so is usb-net. Currently, we couldn't assign default usb address to usb devices, so we definitly ignore it during the assignment of PCI address. Libvirt XML sample: devices interface type='user' mac address='52:54:00:32:6a:91'/ model type='usb-net'/ alias name='net1'/ address type='usb' bus='0' port='1'/ /interface /devices qemu commandline: qemu ${other_vm_args} -netdev user,id=hostnet1 \ -device usb-net,netdev=hostnet1,id=net1,\ mac=52:54:00:32:6a:91,bus=usb.0,port=1 --- docs/formatdomain.html.in | 29 +++-- src/conf/domain_conf.c| 15 --- src/conf/domain_conf.h| 1 + src/qemu/qemu_command.c | 8 +++- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a9003d7..eb7b5dc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2960,24 +2960,17 @@ p For hypervisors which support this, you can set the model of - emulated network interface card. -/p - -p - The values for codetype/code aren't defined specifically by - libvirt, but by what the underlying hypervisor supports (if - any). For QEMU and KVM you can get a list of supported models - with these commands: -/p - -pre -qemu -net nic,model=? /dev/null -qemu-kvm -net nic,model=? /dev/null -/pre - -p - Typical values for QEMU and KVM include: - ne2k_isa i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio + emulated network interface card. For QEMU and KVM, the supported NIC models + are virtio, ne2k_isa, ne2k_pci, pcnet, rtl8139, e1000, usb-net + span class=since(since 1.0.3)/span, spapr-vlan(only PPC64). + For Xen, there are ne2k_isa, ne2k_pci, pcnet, rtl8139, e1000, netfront. For VMWare, + vlance, vmxnet, vmxnet2, vmxnet3 are supported. And for Vbox, Am79C970A, Am79C973, + 82540EM, 82545EM, 82543GC are its supported models. + (All of these models are PCI devices, except usb-net which emulates a Netchip + Linux-USB Ethernet/RNDIS usb ethernet device). For models of PCI type, a sub-element + codelt;addressgt;/code with codetype='pci'/code can be used to tie + the device to a particular PCI slot. It is codetype='usb'/code for usb-net + to tie to USB controller. a href=#elementsAddressdocumented above/a. /p h5a name=elementsDriverBackendOptionsSetting NIC driver-specific options/a/h5 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 96203e2..78a9a77 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -708,6 +708,7 @@ VIR_ENUM_IMPL(virDomainNICModel, VIR_DOMAIN_NIC_MODEL_LAST, default, spapr-vlan, /* qemu */ + usb-net, virtio, /* qemu and vbox */ @@ -5427,15 +5428,16 @@ virDomainNetDefParseXML(virCapsPtr caps, goto error; } -/* XXX what about ISA/USB based NIC models - once we support +/* XXX what about ISA based NIC models - once we support * them we should make sure address type is correct */ if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 -def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Network interfaces must use 'pci' address type)); + _(Network interfaces have incorrect address type)); goto error; } @@ -5642,6 +5644,13 @@ virDomainNetDefParseXML(virCapsPtr caps, } def-driver.virtio.event_idx = idx; } +} else if (def-model == VIR_DOMAIN_NIC_MODEL_USB_NET) { +if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(Interface of usb-net model requires address of usb type)); +goto error; +} } def-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 687032b..8ef044e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -785,6 +785,7 @@ struct _virDomainFSDef { enum virDomainNICModel { VIR_DOMAIN_NIC_MODEL_DEFAULT = 0, VIR_DOMAIN_NIC_MODEL_SPAPR_VLAN, +VIR_DOMAIN_NIC_MODEL_USB_NET, VIR_DOMAIN_NIC_MODEL_VIRTIO, VIR_DOMAIN_NIC_MODEL_NE2K_ISA, VIR_DOMAIN_NIC_MODEL_NE2K_PCI, diff --git a/src/qemu/qemu_command.c
[libvirt] Entering freeze for libvirt-1.0.3
I have just tagged git and pushed the tarball. The rpms for F17 are following too at the usual place: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests, but of course more testing and checking on other architectures is needed ! If everything goes fine, I will make an rc2 in a couple of days and then we can decide if a final release on Friday is fine or if we need to postpone to next week, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Versioning/Compatibility in the XML schemas
Out of curiosity, seeing that we doing modifications to the XML schema all the time, there does not seem to be some sort of version identifier in the XML schemas. If I take the recently merged patch for DHCP options, for instance, suppose virt-manager would add support for it in its GUI. How would such a network XML definition be interpreted by older libvirt versions? Is it supposed to ignore elements it doesn't know or do we have an implicit requirement of always use the latest versions together? -- Pieter Hollants pie...@hollants.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Versioning/Compatibility in the XML schemas
On Mon, Feb 25, 2013 at 01:13:37PM +0100, Pieter Hollants wrote: Out of curiosity, seeing that we doing modifications to the XML schema all the time, there does not seem to be some sort of version identifier in the XML schemas. If I take the recently merged patch for DHCP options, for instance, suppose virt-manager would add support for it in its GUI. How would such a network XML definition be interpreted by older libvirt versions? Is it supposed to ignore elements it doesn't know or do we have an implicit requirement of always use the latest versions together? The base principle is that we are supposed to only do additions i.e. an XML which was RNG valid for a version A should still be valid for the RHN shipped with version B , B = A. And an older version of the code usually still work with a newer XML instance. There had been some small breakage IIRC but usually thise rule works. In general the way it works is that we don't validate XML input, but we convert it to an internal data structure by picking the information we know about. So the parser from A may extract only the subset it knows about. The danger is that it will accept XML definition without fully understanding the semantic of all elements, instead of filing to load it. It is usually acceptable, the problem is for example when the discarded elements includes security or features really needed for proper work. One thing which will fail commonly is that if we add an extra value for an attribute, usually we use macro mapping the text value in the XML to an internal enum, and if the match can't be done, you get an error. So we tend to fail on unknow attribute values, but accept unknown element most of the time. Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ 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] [PATCHv2 0/5] Add API to allow TCP connection tunelling
Hi Peter, On Mon, Dec 10, 2012 at 09:29:39AM +0100, Peter Krempa wrote: This series adds ability for the qemu driver to tunnel connections to TCP ports from the host. This is useful for enabling remote VNC/SPICE sessions without the need to configure SSH tunnels or portforwards and without the need to open the ports for public. It'd be nice to have this in. Are you still on it? Cheers, -- Guido There's also an advantage for tools such as virt-viewer that have to guess the remote connection parameters and you have to hope that nothing is in your way. With spice/VNC clients that have support for read/write callbacks, this would allow also direct connection without an intermediate socket. The API and tunelling works but there's no (stable and good working) client for this API. I hacked up a dirty netcat-like terminal into virsh for testing purposes (see patch 5/5) but that isn't what I'd like to see. The client should be able to open a listening socket and when a client connects to it, it opens a stream and connects it to the remote host. For the client there are two options: 1) do all the stuff in virsh: + one tool to rule them all - i'd like to daemonize it and I don't know if that's okay in virsh 2) add a new tool virtunnel: + less virsh pollution - separate tool ... As nobody responded, I'd like to re-ask for someones opinion on this. (note: this is my personal effort, I'm annoyed of opening ssh tunnels to remote displays on my server and I don't want to open the ports to public. ) After this it would be great to add support for this to virt-viewer. I will have a look at that later. Diff to v1: - fixed error reporting in 2/5 - documented limitation to localhost in 3/5 - fixed possible segfault in 4/5 --- Peter Krempa (5): api: Add API to allow TCP tunneling through streams to the host fdstream: Add support for TCP connections of streams qemu: Add configuration options to enable TCP tunelling qemu: Implement virNodeTunnelTcp for the qemu driver NOT_TO_BE_APPLIED_UPSTREAM: quick and dirty virsh client to test the stuff include/libvirt/libvirt.h.in | 11 +++ src/driver.h | 8 + src/fdstream.c | 73 src/fdstream.h | 5 +++ src/libvirt.c| 67 src/libvirt_private.syms | 1 + src/libvirt_public.syms | 1 + src/qemu/qemu.conf | 16 ++ src/qemu/qemu_conf.c | 26 src/qemu/qemu_conf.h | 13 src/qemu/qemu_driver.c | 48 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 9 +- src/remote_protocol-structs | 6 src/rpc/gendispatch.pl | 1 + tools/console.c | 66 +-- tools/console.h | 9 ++ tools/virsh-domain.c | 17 +-- tools/virsh-host.c | 60 19 files changed, 397 insertions(+), 41 deletions(-) -- 1.8.0 -- 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 1/2] Trivial fix: in dhcp-host the name is optional
On 02/19/2013 04:39 PM, Eric Blake wrote: On 02/15/2013 12:02 PM, Gene Czarcinski wrote: Although in IPv4 one must pick either mac or name, either can be omitted. Similarly, for IPv6, the name can be optionally omitted. Signed-off-by: Gene Czarcinski g...@czarc.net --- docs/schemas/network.rng | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 09d7c73..a479453 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -281,7 +281,9 @@ optional attribute name=macref name=uniMacAddr//attribute /optional -attribute name=nametext//attribute +optional + attribute name=nametext//attribute +/optional Hmm. This says that I can omit both name and mac, and still validate. Better would be this RelaxNG construct, which says that I must supply at least one, but can also supply both; the difference is that failure to supply either will cause a (desired) validation failure. choice group attribute name=macref name=uniMacAddr//attribute optional attribute name=nametext//attribute /optional /group attribute name=nametext//attribute /choice I know Laine already ack'd but since we haven't pushed yet, I'm wondering if it is worth tightening the grammar. First of all, I like your suggestion since it is closer to reality. However, when I posed the question here, this was my answer: On 02/18/2013 09:12 AM, Daniel P. Berrange wrote: The schemas are intended to provide a simple syntax check for XML documents that an app has created. eg detect typos in element/attribute names, or use of elements which don't use or incorrect nesting of elements. Even for syntax checks though, the schemas a not going to provide 100% coverage - todo so would make the schemas far more complicated and it is just not worth the effort. The XML parser is the ultimate place where 100% complete syntax checks are performed. What they generally do not do, is provide any kind of semantic check about whether the configuration actually makes sense. For example, the schema won't tell you that requesting virtio disks with a vmware guest is semantically invalid. This is left to the virt driver. So, my interpretation is that the schema should not call false negatives but getting all the positives correct is not necessary. This whole thing with dhcp-host is further complicated by IPv6. That is, for IPv4 you need MAC whereas for IPv6 you need (client) ID but not MAC. Supposedly, for IPv4 you could specify client-id but I have not experimented with that. In both IPv4 and IPv6, my experiments with dnsmasq say that specifying a name is not sufficient to have an IP address assigned whereas for IPv4 specifying a MAC and for IPv6 specifying an ID is sufficient. Gene -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/5] Add API to allow TCP connection tunelling
On 02/25/13 14:51, Guido Günther wrote: Hi Peter, On Mon, Dec 10, 2012 at 09:29:39AM +0100, Peter Krempa wrote: This series adds ability for the qemu driver to tunnel connections to TCP ports from the host. This is useful for enabling remote VNC/SPICE sessions without the need to configure SSH tunnels or portforwards and without the need to open the ports for public. It'd be nice to have this in. Are you still on it? Cheers, -- Guido I'm planing to respin this in some time with a few changes. I will probably not allow to tunnel arbitrary ports from the machine libvirt is running on but limit it to a few selected ones controllable by flags. (The general approach did not get accepted very well) Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: enable direct migration over IPv6
Use virURIParse in qemuMigrationPrepareDirect to allow parsing IPv6 addresses, which would cause an 'incorrect :port' error message before. To be able to migrate over IPv6, QEMU needs to listen on [::] instead of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen address based on the result. It also uses the same listen address for the NBD server. This will break migration if a hostname does not resolve to the same address family on both sides. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013 --- Diff to V1: * initialize uri_str * reuse STRSKIP(tcp:) result instead of doing strlen on it * print a warning instead of failing when the hostname can't be resolved Diff to V2: * freeaddrinfo * separate the listen address to allow reuse in qemuMigrationStartNBDServer src/qemu/qemu_migration.c | 77 --- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index cae58fa..ff9b959 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -22,7 +22,10 @@ #include config.h +#include netdb.h +#include sys/socket.h #include sys/time.h +#include sys/types.h #ifdef WITH_GNUTLS # include gnutls/gnutls.h # include gnutls/x509.h @@ -1103,12 +1106,12 @@ error: */ static int qemuMigrationStartNBDServer(virQEMUDriverPtr driver, -virDomainObjPtr vm) +virDomainObjPtr vm, +const char *listenAddr) { int ret = -1; qemuDomainObjPrivatePtr priv = vm-privateData; unsigned short port = 0; -const char *listenAddr = 0.0.0.0; char *diskAlias = NULL; size_t i; @@ -1981,6 +1984,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, const char *dom_xml, const char *migrateFrom, virStreamPtr st, +const char *listenAddr, unsigned long flags) { virDomainDefPtr def = NULL; @@ -2168,7 +2172,7 @@ done: if (flags VIR_MIGRATE_TUNNELLED) VIR_DEBUG(NBD in tunnelled migration is currently not supported); else { -if (qemuMigrationStartNBDServer(driver, vm) 0) { +if (qemuMigrationStartNBDServer(driver, vm, listenAddr) 0) { /* error already reported */ goto endjob; } @@ -2271,7 +2275,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, */ ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, dname, dom_xml, - stdio, st, flags); + stdio, st, NULL, flags); return ret; } @@ -2292,9 +2296,14 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, static int port = 0; int this_port; char *hostname = NULL; +char listenAddr[8]; char migrateFrom [64]; const char *p; +char *uri_str = NULL; int ret = -1; +bool ipv6 = false; +struct addrinfo *info; +virURIPtr uri; VIR_DEBUG(driver=%p, dconn=%p, cookiein=%s, cookieinlen=%d, cookieout=%p, cookieoutlen=%p, uri_in=%s, uri_out=%p, @@ -2343,16 +2352,39 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, * URI when passing it to the qemu monitor, so bad * characters in hostname part don't matter. */ -if (!STRPREFIX(uri_in, tcp:)) { +if (!(p = STRSKIP(uri_in, tcp:))) { virReportError(VIR_ERR_INVALID_ARG, %s, _(only tcp URIs are supported for KVM/QEMU migrations)); goto cleanup; } -/* Get the port number. */ -p = strrchr(uri_in, ':'); -if (p == strchr(uri_in, ':')) { +/* Convert uri_in to well-formed URI with // after tcp: */ +if (!(STRPREFIX(uri_in, tcp://))) { +if (virAsprintf(uri_str, tcp://%s, p) 0) { +virReportOOMError(); +goto cleanup; +} +} + +uri = virURIParse(uri_str ? uri_str : uri_in); +VIR_FREE(uri_str); + +if (uri == NULL) { +virReportError(VIR_ERR_INVALID_ARG, _(unable to parse URI: %s), + uri_in); +goto cleanup; +} + +if (uri-server == NULL) { +virReportError(VIR_ERR_INVALID_ARG, _(missing host in migration + URI: %s), uri_in); +goto cleanup; +} else { +hostname = uri-server; +} + +if (uri-port == 0) { /* Generate a port */ this_port = QEMUD_MIGRATION_FIRST_PORT + port++; if (port == QEMUD_MIGRATION_NUM_PORTS) @@ -2365,25 +2397,34 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
Re: [libvirt] [PATCHv2 3/8] conf: Add support for RNG device configuration in XML
On 02/25/13 10:42, Peter Krempa wrote: On 02/23/13 00:33, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: This patch adds basic configuration support for the RNG device suporting s/suporting/supporting/ the virtio model with the random and egd backend types as described in the schema in the previous patch. --- Notes: Version 2: - fix a ton of memory leaks (I assumed that virXMLGetProp returns static strings) - Add new device type to even more places - Fix error message cp error - Fix memleak in RNGDef free func ... @@ -10601,6 +10734,22 @@ virDomainDefParseXML(virCapsPtr caps, } } +/* Parse the RNG device */ +if ((n = virXPathNodeSet(./devices/rng, ctxt, nodes)) 0) +goto error; + +if (n 1) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(only a single RNG device is supported)); Is this an inherent limit of qemu? For that matter, is it an inherent limit, and no hypervisor can ever support more than one? In the bare metal case, can't you plug in multiple rng hardware dongles? AFAIK qemu is able to support multiple RNG devices but I didn't find that particularly useful. I will follow up with a patch that will add support for more than one RNG device, although I don't think it will be used much. I hacked up the patch to add support for multiple RNG's. qemu appears to support the device just fine. As of guest support it seems that it triggers a 100% reproducible kernel panic on linux guest when attempting to read from a machine with two virtio-rng devices. I filed a bug to track this issue: https://bugzilla.redhat.com/show_bug.cgi?id=915335 Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] storage: fix uid_t|gid_t handling on 32 bit Linux
On 22.02.2013 17:55, Philipp Hahn wrote: uid_t and gid_t are opaque types, ranging from s32 to u32 to u64. Unfortunately libvirt uses the value -1 to mean current process, which on Linux32 gets converted to ALLONE := +(2^32-1) = 4294967295. Different libvirt versions used different formatting in the past, which break one or the other parsing function: virXPathLong(): (signed long)-1 != (double)ALLONE virXPathULong(): (unsigned long)-1 != (double)-1 Roll our own version of virXPathULong(), which also accepts -1, which is silently converted to ALLONE. For output: do the reverse and print -1 instead of ALLONE. Signed-off-by: Philipp Hahn h...@univention.de --- src/conf/storage_conf.c | 74 ++- 1 file changed, 60 insertions(+), 14 deletions(-) This one breaks storagevolxml2xmltest: TEST: storagevolxml2xmltest 1) Storage Vol XML-2-XML vol-file... OK 2) Storage Vol XML-2-XML vol-file-backing... OK 3) Storage Vol XML-2-XML vol-qcow2 ... OK 4) Storage Vol XML-2-XML vol-partition ... OK 5) Storage Vol XML-2-XML vol-logical ... OK 6) Storage Vol XML-2-XML vol-logical-backing ... OK 7) Storage Vol XML-2-XML vol-sheepdog... Offset 283 Expect [4294967295/owner group4294967295] Actual [-1/owner group-1] ... FAILED However, the first 3 patches are correct. Even though it's freeze, they are pure bugfixes so I've pushed them. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH] vl.c: Support multiple CPU ranges on -numa option
On Fri, Feb 22, 2013 at 11:04:24AM +0100, Markus Armbruster wrote: Markus Armbruster arm...@redhat.com writes: Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster arm...@redhat.com writes: Eduardo Habkost ehabk...@redhat.com writes: This allows , to be used a separator between each CPU range. Note that commas inside key=value command-line options have to be escaped using ,,, so the command-line will look like: -numa node,cpus=A,,B,,C,,D This is really, really ugly, and an embarrassment to document. Which you didn't ;) What about -numa node,cpus=A,cpus=B,cpus=C,cpus=D Yes, QemuOpts lets you do that. Getting all the values isn't as easy as it could be (unless you use Laszlo's opt-visitor), but that could be improved. No more of this. -numa node,cpus=A:B:C:D if you want to express a list. Okay for command line and human monitor, just don't let it bleed into QMP. Footnotes: 1. Using colons for lists works only as long as the list elements don't contain colons. Fine for numbers. No good for filenames, network addresses, ... 2. QemuOpts helped us reduce the number of ad hoc option parsers, improving consistency and error messages quite a bit. Having every user of colon lists roll their own ad hoc parser slides back into the hole that motivated QemuOpts. Let's try to avoid that, please. 3. The existing QemuOpts syntax for list-valued options (repeating the option) doesn't have either of these problems. The problem here seems to be that we want to reuse option parsing code, but the only reusable syntax we have for command-line options today is awful (at least for representing lists). So our only options seem to be: 1) accept some ugliness and things like A,,B,,C or cpus=A,cpus=B,cpus=C; 2) write ad hoc option parsers; 3) define/choose a new reusable syntax. We already have at least 2 better ways to represent config data (config files and QMP+JSON), but why do we insist in using command-line options with an awful syntax for everything? -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: do not set unpriv_sgio if neither supported nor requested
Currently we call virSetDeviceUnprivSGIO with val == 0 if a block device has an sgio attribute. But for sgio='filtered', we know that a kernel with no unpriv_sgio support will always behave as the user wanted. In this case, there is no need to call the function and report a (bogus) error. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_process.c | 35 +-- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b560d2e..4c152b2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3455,36 +3455,35 @@ qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver) int qemuSetUnprivSGIO(virDomainDiskDefPtr disk) { +char *sysfs_path = NULL; int val = -1; +int ret = 0; /* sgio is only valid for block disk; cdrom * and floopy disk can have empty source. */ if (disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK || +disk-device != VIR_DOMAIN_DISK_DEVICE_LUN || !disk-src) return 0; -if (disk-sgio) -val = (disk-sgio == VIR_DOMAIN_DISK_SGIO_UNFILTERED); - -/* Ignore the setting if unpriv_sgio is not supported by the - * kernel, otherwise defaults to filter the SG_IO commands, - * I.E. Set unpriv_sgio to 0. - */ -if (disk-sgio == VIR_DOMAIN_DISK_SGIO_DEFAULT -disk-device == VIR_DOMAIN_DISK_DEVICE_LUN) { -char *sysfs_path = NULL; +sysfs_path = virGetUnprivSGIOSysfsPath(disk-src, NULL); +if (sysfs_path == NULL) +return -1; -if ((sysfs_path = virGetUnprivSGIOSysfsPath(disk-src, NULL)) -virFileExists(sysfs_path)) -val = 0; -VIR_FREE(sysfs_path); -} +/* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0. */ +val = (disk-sgio == VIR_DOMAIN_DISK_SGIO_UNFILTERED); -if (val = 0 virSetDeviceUnprivSGIO(disk-src, NULL, val) 0) -return -1; +/* Do not do anything if unpriv_sgio is not supported by the kernel and the + * whitelist is enabled. But if requesting unfiltered access, always call + * virSetDeviceUnprivSGIO, to report an error for unsupported unpriv_sgio. + */ +if ((virFileExists(sysfs_path) || val == 1) +virSetDeviceUnprivSGIO(disk-src, NULL, val) 0) +ret = -1; -return 0; +VIR_FREE(sysfs_path); +return ret; } int qemuProcessStart(virConnectPtr conn, -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] storage: cast -1 for uid_t|gid_t
On 02/22/2013 09:41 AM, Philipp Hahn wrote: uid_t and gid_t are opaque types, ranging from s32 to u32 to u64. Or even s16 and u16 in older Unix. Explicitly cast the magic -1 to the appropriate type. Necessary in some cases, redundant in others; but overall good to be consistent. Signed-off-by: Philipp Hahn h...@univention.de --- src/conf/storage_conf.c |8 src/storage/storage_backend.c | 16 src/util/virutil.c|2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7a39998..9134a22 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -674,8 +674,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, if (node == NULL) { /* Set default values if there is not permissions element */ perms-mode = defaultmode; -perms-uid = -1; -perms-gid = -1; +perms-uid = (uid_t) -1; +perms-gid = (gid_t) -1; Redundant. C guarantees that -1 will promote correctly via assignment to any size integer, signed or unsigned. perms-label = NULL; return 0; } @@ -700,7 +700,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, } if (virXPathNode(./owner, ctxt) == NULL) { -perms-uid = -1; +perms-uid = (uid_t) -1; Redundant. } else { if (virXPathLong(number(./owner), ctxt, v) 0) { virReportError(VIR_ERR_XML_ERROR, @@ -711,7 +711,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, } if (virXPathNode(./group, ctxt) == NULL) { -perms-gid = -1; +perms-gid = (gid_t) -1; Redundant. } else { if (virXPathLong(number(./group), ctxt, v) 0) { virReportError(VIR_ERR_XML_ERROR, diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 964ce29..ec2fa53 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -277,9 +277,9 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED, vol-target.path); goto cleanup; } -uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : -1; -gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : -1; -if (((uid != -1) || (gid != -1)) +uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : (uid_t) -1; Necessary - ternary operator rules for integer promotion are not necessarily intuitive; and it is conceivable that mixing u16 with int can produce wrong results. Forcing both sides of the expression to be uid_t is indeed worthwhile. +gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : (gid_t) -1; Ditto on being necessary. +if (((uid != (uid_t) -1) || (gid != (gid_t) -1)) And comparison also needs the cast - definitely necessary. (fchown(fd, uid, gid) 0)) { virReportSystemError(errno, _(cannot chown '%s' to (%u, %u)), @@ -542,9 +542,9 @@ static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, if ((pool-def-type == VIR_STORAGE_POOL_NETFS) (((getuid() == 0) - (vol-target.perms.uid != -1) + (vol-target.perms.uid != (uid_t) -1) (vol-target.perms.uid != 0)) -|| ((vol-target.perms.gid != -1) +|| ((vol-target.perms.gid != (gid_t) -1) Necessary. (vol-target.perms.gid != getgid() { virCommandSetUID(cmd, vol-target.perms.uid); @@ -572,9 +572,9 @@ static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, } } -uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : -1; -gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : -1; -if (((uid != -1) || (gid != -1)) +uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : (uid_t) -1; +gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : (gid_t) -1; +if (((uid != (uid_t) -1) || (gid != (gid_t) -1)) Necessary. (chown(vol-target.path, uid, gid) 0)) { virReportSystemError(errno, _(cannot chown %s to (%u, %u)), diff --git a/src/util/virutil.c b/src/util/virutil.c index 563f684..4af2599 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1206,7 +1206,7 @@ parenterror: _(stat of '%s' failed), path); goto childerror; } -if ((st.st_gid != gid) (chown(path, -1, gid) 0)) { +if ((st.st_gid != gid) (chown(path, (uid_t) -1, gid) 0)) { Redundant (integer promotion via prototyped function calls does the right thing). Since the patch is already pushed, I'm fine leaving the redundant parts in. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library
Re: [libvirt] RFC: APIs for managing resource groups
On Mon, Feb 25, 2013 at 09:48:16AM -0700, Eric Blake wrote: On 02/25/2013 05:41 AM, Daniel P. Berrange wrote: Historically for the QEMU/LXC drivers we've simply put each virtual instance in a dedicated cgroup, under the path We need to simplify our layout and also introduce some APIs for the grouping of VMs. I won't go into specifics of a new cgroups layout here, just focus on the question of defining a set of APIs that are generic to any hypervisor, for the purpose of setting up VM resource groups. I'm very much in favor of VM resource groups. In fact, this RFC has come up in the past, if it gives you any ideas of what you replied back then: https://www.redhat.com/archives/libvir-list/2011-March/msg01546.html I'm calling the resource cgroup a partition, since this is all about partitioning workloads. Yes, that naming is workable, and a bit better than what I tried last time. I anticipate a new top level object and APIs for creating/defining it in the usual manner: snip good API There'd also likely be a new VM XML element partition name=..partition name.../ which is what the Get/SetPartition methods would be touching. Earlier, you pointed out that it might make sense to have multiple partitions per domain - that is, have one partitioning that controls only memory usage, and another partitioning that controls only cpu usage, then have a domain that belongs to two orthogonal partitions to cap both memory and cpu. Your proposal today doesn't seem to deal with the idea of having multiple partitions per domain. Also, while you proposed having a domain belong to a partition, you didn't cover whether it makes sense to have a hierarchy of partitions, where one partition can provide further constraints on top of a parent partition. While cgroups currently allows you to setup different hiearchies for memory, cpu, blockio, etc controls, these days it is agreed that this is an anti-feature causing no end of trouble for all involved. In the future I expect the kernel will enforce that we use the same hierarchy for all cgroup controllers. In other words, we only want a single partition for all resources. I do anticipate that we'll be able to create partition hierarchies, though it may be discouraged since it has performance implications for the kernel that are currently somewhat unacceptable. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH tck] Avoid multicast MAC addrs in tests
From: Daniel P. Berrange berra...@redhat.com Certain MAC addrs are used for multicast and recent libvirt will refuse to start VMs using those addrs. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- scripts/domain/215-nic-hotplug-many.t | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/domain/215-nic-hotplug-many.t b/scripts/domain/215-nic-hotplug-many.t index 5b935cc..8b4afbd 100644 --- a/scripts/domain/215-nic-hotplug-many.t +++ b/scripts/domain/215-nic-hotplug-many.t @@ -47,9 +47,9 @@ diag Creating a new transient domain; my $dom; ok_domain(sub { $dom = $conn-create_domain($xml) }, created transient domain object); -my $mac1 = 01:11:22:33:44:55; -my $mac2 = 02:11:22:33:44:55; -my $mac3 = 03:11:22:33:44:55; +my $mac1 = 02:11:22:33:44:51; +my $mac2 = 02:11:22:33:44:52; +my $mac3 = 02:11:22:33:44:53; my $model = virtio; my $netxml1 = EOF; -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: APIs for managing resource groups
On 02/25/2013 05:41 AM, Daniel P. Berrange wrote: Historically for the QEMU/LXC drivers we've simply put each virtual instance in a dedicated cgroup, under the path We need to simplify our layout and also introduce some APIs for the grouping of VMs. I won't go into specifics of a new cgroups layout here, just focus on the question of defining a set of APIs that are generic to any hypervisor, for the purpose of setting up VM resource groups. I'm very much in favor of VM resource groups. In fact, this RFC has come up in the past, if it gives you any ideas of what you replied back then: https://www.redhat.com/archives/libvir-list/2011-March/msg01546.html I'm calling the resource cgroup a partition, since this is all about partitioning workloads. Yes, that naming is workable, and a bit better than what I tried last time. I anticipate a new top level object and APIs for creating/defining it in the usual manner: snip good API There'd also likely be a new VM XML element partition name=..partition name.../ which is what the Get/SetPartition methods would be touching. Earlier, you pointed out that it might make sense to have multiple partitions per domain - that is, have one partitioning that controls only memory usage, and another partitioning that controls only cpu usage, then have a domain that belongs to two orthogonal partitions to cap both memory and cpu. Your proposal today doesn't seem to deal with the idea of having multiple partitions per domain. Also, while you proposed having a domain belong to a partition, you didn't cover whether it makes sense to have a hierarchy of partitions, where one partition can provide further constraints on top of a parent partition. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] storage: Cast uid_t|gid_t to unsigned int
On 02/22/2013 09:43 AM, Philipp Hahn wrote: uid_t and gid_t are opaque types, ranging from s32 to u32 to u64. Technically, I've never seen a platform with uid_t or gid_t being u64 - that's still theoretical at this time. However, since pid_t is 64-bit on mingw64, and since id_t is the union of [pug]id_t and therefore also a 64-bit type, I agree with this commit. Note that we have a compile-time assertion that on all platforms where libvirt is currently compiling, that sizeof(uid_t) = sizeof(int) (likewise for gid_t). We'd have a LOT more code to clean up if we ever encounter a platform with u64 [ug]id_t. Still, since your commit has already been pushed, we can't touch up the commit message now. And ACK to your change. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Refactor qemuDomainSetMemoryParameters
On 02/22/13 13:56, John Ferlan wrote: On 02/18/2013 10:18 AM, Peter Krempa wrote: The new TypedParam helper APIs allow to simplify this function significantly. This patch integrates the fix in 75e5bec97b3045e4f926248d5c742f8a50d0f9 by correctly ordering the setting functions instead of reordering the parameters. --- src/qemu/qemu_driver.c | 149 ++--- 1 file changed, 54 insertions(+), 95 deletions(-) ... /* It will fail if hard limit greater than swap hard limit anyway */ -if (swap_hard_limit -hard_limit -(hard_limit-value.ul swap_hard_limit-value.ul)) { +if (set_swap_hard_limit set_memory_hard_limit +(memory_hard_limit swap_hard_limit)) { virReportError(VIR_ERR_INVALID_ARG, %s, _(hard limit must be lower than swap hard limit)); Consider the messages below, should this be memory hard_limit tunable value must be lower than swap_hard_limit I went with the modified spelling of the message ... ... ACK - seems reasonable to me - your choice on the error message ... and pushed. Thanks for the review. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix crash changing CDROM media
From: Daniel P. Berrange berra...@redhat.com This change tried to fix a crash with changing CDROM media but failed to actually do so commit d0172d2b1b5d865aaa042070d7c2d00effb2ff8c Author: Osier Yang jy...@redhat.com Date: Tue Feb 19 20:27:45 2013 +0800 qemu: Remove the shared disk entry if the operation is ejecting or updating It was still accessing disk-src, when the entire 'disk' object has been free'd already. Even if it weren't free'd, accessing the 'src' value of virDomainDiskDef is not allowed without first validating disk-type is file or block. Just remove the broken code entirely. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_driver.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e96915..8dae8f9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5778,13 +5778,14 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, dev-data.disk = tmp; ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, false); +/* 'disk' must not be accessed now - it has been free'd. + * 'orig_disk' now points to the new disk, while 'dev_copy' + * now points to the old disk */ /* Need to remove the shared disk entry for the original disk src * if the operation is either ejecting or updating. */ -if (ret == 0 -orig_disk-src -STRNEQ_NULLABLE(orig_disk-src, disk-src)) +if (ret == 0) ignore_value(qemuRemoveSharedDisk(driver, dev_copy-data.disk, vm-def-name)); break; -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/13] qemu: support named nbd exports
These are supported by nbd-server and by the NBD server that QEMU embeds for live image access. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/formatdomain.html.in | 6 ++-- src/qemu/qemu_command.c| 17 +++ tests/qemuargv2xmltest.c | 1 + ...qemuxml2argv-disk-drive-network-nbd-export.args | 5 .../qemuxml2argv-disk-drive-network-nbd-export.xml | 33 ++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c| 1 + 7 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a9003d7..90cfc03 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1441,9 +1441,9 @@ are nbd, rbd, sheepdog or gluster. If the codeprotocol/code attribute is rbd, sheepdog or gluster, an additional attribute codename/code is mandatory to specify which -volume/image will be used. When the disk codetype/code is -network, the codesource/code may have zero or -more codehost/code sub-elements used to specify the hosts +volume/image will be used; for nbd it is optional. When the disk +codetype/code is network, the codesource/code may have zero +or more codehost/code sub-elements used to specify the hosts to connect. span class=sinceSince 0.0.3; codetype='dir'/code since 0.7.5; codetype='network'/code since 0.8.7/spanbr/ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index beb7cfe..4762f0a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2132,6 +2132,7 @@ qemuParseNBDString(virDomainDiskDefPtr disk) { virDomainDiskHostDefPtr h = NULL; char *host, *port; +char *src; if (VIR_ALLOC(h) 0) goto no_memory; @@ -2149,11 +2150,24 @@ qemuParseNBDString(virDomainDiskDefPtr disk) if (!h-name) goto no_memory; +src = strchr(port, ':'); +if (src) +*src++ = '\0'; + h-port = strdup(port); if (!h-port) goto no_memory; +if (src STRPREFIX(src, exportname=)) { +src = strdup(strchr(src, '=') + 1); +if (!src) +goto no_memory; +} else { +src = NULL; +} + VIR_FREE(disk-src); +disk-src = src; disk-nhosts = 1; disk-hosts = h; return 0; @@ -2254,6 +2268,9 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) break; } +if (disk-src) +virBufferEscape(opt, ',', ,, :exportname=%s, disk-src); + return 0; } diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 3c23010..1731022 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -182,6 +182,7 @@ mymain(void) DO_TEST(disk-drive-cache-directsync); DO_TEST(disk-drive-cache-unsafe); DO_TEST(disk-drive-network-nbd); +DO_TEST(disk-drive-network-nbd-export); DO_TEST(disk-drive-network-gluster); DO_TEST(disk-drive-network-rbd); DO_TEST(disk-drive-network-rbd-ipv6); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args new file mode 100644 index 000..bf411ff --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +file=nbd:example.org:6000:exportname=bar,if=virtio,format=raw -net none -serial none \ +-parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml new file mode 100644 index 000..f2b5ca4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml @@ -0,0 +1,33 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +disk type='network'
[libvirt] [PATCH 02/13] qemu: do not support non-network disks without -drive
QEMU added -drive in 2007, and NBD in 2008. Both appeared first in release 0.10.0. Thus the code to support network disks without -drive is dead, and in fact it incorrectly escapes commas. Drop it. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_command.c | 53 ++--- 1 file changed, 2 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5dccaae..a3c5a4e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5901,57 +5901,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto no_memory; } } else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) { -switch (disk-protocol) { -case VIR_DOMAIN_DISK_PROTOCOL_NBD: -if (disk-nhosts != 1) { -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(NBD accepts only one host)); -goto error; -} -if (virAsprintf(file, nbd:%s:%s,, disk-hosts-name, -disk-hosts-port) 0) { -goto no_memory; -} -break; -case VIR_DOMAIN_DISK_PROTOCOL_RBD: -{ -virBuffer opt = VIR_BUFFER_INITIALIZER; -if (qemuBuildRBDString(conn, disk, opt) 0) -goto error; -if (virBufferError(opt)) { -virReportOOMError(); -goto error; -} -file = virBufferContentAndReset(opt); -} -break; -case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: -{ -virBuffer opt = VIR_BUFFER_INITIALIZER; -if (qemuBuildGlusterString(disk, opt) 0) -goto error; -if (virBufferError(opt)) { -virReportOOMError(); -goto error; -} -file = virBufferContentAndReset(opt); -} -break; -case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: -if (disk-nhosts == 0) { -if (virAsprintf(file, sheepdog:%s,, disk-src) 0) { -goto no_memory; -} -} else { -/* only one host is supported now */ -if (virAsprintf(file, sheepdog:%s:%s:%s,, -disk-hosts-name, disk-hosts-port, -disk-src) 0) { -goto no_memory; -} -} -break; -} +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(network disks are only supported with -drive)); } else { if (!(file = strdup(disk-src))) { goto no_memory; -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/13] qemu: add support for libiscsi
libiscsi provides a userspace iSCSI initiator. The main advantage over the kernel initiator is that it is very easy to provide different initiator names for VMs on the same host. Thus libiscsi supports usage of persistent reservations in the VM, which otherwise would only be possible with NPIV. libiscsi uses iscsi as the scheme, not iscsi+tcp. We can change this in the tests (while remaining backwards-compatible manner, because QEMU uses TCP as the default transport for both Gluster and NBD). Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_command.c| 49 +- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-gluster.args | 2 +- .../qemuxml2argv-disk-drive-network-iscsi.args | 1 + ...ml2argv-disk-drive-network-nbd-ipv6-export.args | 2 +- .../qemuxml2argv-disk-drive-network-nbd-ipv6.args | 2 +- tests/qemuxml2argvtest.c | 2 + 7 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 733d3bf..07700cc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2145,6 +2145,23 @@ qemuParseGlusterString(virDomainDiskDefPtr def) } static int +qemuParseISCSIString(virDomainDiskDefPtr def) +{ +virURIPtr uri = NULL; + +if (!(uri = virURIParse(def-src))) +return -1; + +if (uri-path strchr(uri-path + 1, '/')) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(invalid address for iSCSI target), disk-src); +return -1; +} + +return qemuParseDriveURIString(def, uri, iscsi); +} + +static int qemuParseNBDString(virDomainDiskDefPtr disk) { virDomainDiskHostDefPtr h = NULL; @@ -2238,8 +2255,14 @@ qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt, virBufferAddLit(opt, file=); transp = virDomainDiskProtocolTransportTypeToString(disk-hosts-transport); -if (virAsprintf(tmpscheme, %s+%s, scheme, transp) 0) -goto no_memory; +if (disk-hosts-transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) { +tmpscheme = strdup(scheme); +if (tmpscheme == NULL) +goto no_memory; +} else { +if (virAsprintf(tmpscheme, %s+%s, scheme, transp) 0) +goto no_memory; +} if (disk-src virAsprintf(volimg, /%s, disk-src) 0) goto no_memory; @@ -2283,6 +2306,12 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) } static int +qemuBuildISCSIString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ +return qemuBuildDriveURIString(disk, opt, iscsi); +} + +static int qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) { const char *transp; @@ -2471,6 +2500,11 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; virBufferAddChar(opt, ','); break; +case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: +if (qemuBuildISCSIString(disk, opt) 0) +goto error; +virBufferAddChar(opt, ','); +break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: if (disk-nhosts == 0) { @@ -7503,6 +7537,12 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps, if (qemuParseGlusterString(def) 0) goto error; +} else if (STRPREFIX(def-src, iscsi:)) { +def-type = VIR_DOMAIN_DISK_TYPE_NETWORK; +def-protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; + +if (qemuParseISCSIString(def) 0) +goto error; } else if (STRPREFIX(def-src, sheepdog:)) { char *p = def-src; char *port, *vdi; @@ -8793,6 +8833,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto error; break; +case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: +if (qemuParseISCSIString(disk) 0) +goto error; + +break; } } diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 9fbba94..314c7b3 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -186,6 +186,7 @@ mymain(void) DO_TEST(disk-drive-network-nbd-ipv6); DO_TEST(disk-drive-network-nbd-ipv6-export); DO_TEST(disk-drive-network-nbd-unix); +DO_TEST(disk-drive-network-iscsi); DO_TEST(disk-drive-network-gluster); DO_TEST(disk-drive-network-rbd); DO_TEST(disk-drive-network-rbd-ipv6); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args index 6dbb009..9d70586 100644 ---
[libvirt] [PATCH 05/13] qemu: support NBD with Unix sockets
This reuses the XML format that was introduced for Gluster. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/formatdomain.html.in | 8 ++-- src/qemu/qemu_command.c| 49 +++--- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-nbd-unix.args | 5 +++ .../qemuxml2argv-disk-drive-network-nbd-unix.xml | 33 +++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c| 1 + 7 files changed, 80 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 90cfc03..094b509 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1713,9 +1713,11 @@ td only one /td /tr /table -In case of gluster, valid values for transport attribute are tcp, rdma -or unix. If nothing is specified, tcp is assumed. If transport is unix, -socket attribute specifies path to unix socket. +gluster supports tcp, rdma, unix as valid values for the +transport attribute. nbd supports tcp and unix. Others only +support tcp. If nothing is specified, tcp is assumed. If the +transport is unix, the socket attribute specifies the path to an +AF_UNIX socket. /dd dtcodeaddress/code/dt ddIf present, the codeaddress/code element ties the disk diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4762f0a..89cd065 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2138,25 +2138,34 @@ qemuParseNBDString(virDomainDiskDefPtr disk) goto no_memory; host = disk-src + strlen(nbd:); -port = strchr(host, ':'); -if (!port) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(cannot parse nbd filename '%s'), disk-src); -goto error; -} +if (STRPREFIX(host, unix:/)) { +src = strchr(host + strlen(unix:), ':'); +if (src) +*src++ = '\0'; -*port++ = '\0'; -h-name = strdup(host); -if (!h-name) -goto no_memory; +h-transport = VIR_DOMAIN_DISK_PROTO_TRANS_UNIX; +h-socket = strdup(host + strlen(unix:)); +} else { +port = strchr(host, ':'); +if (!port) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(cannot parse nbd filename '%s'), disk-src); +goto error; +} -src = strchr(port, ':'); -if (src) -*src++ = '\0'; +*port++ = '\0'; +h-name = strdup(host); +if (!h-name) +goto no_memory; -h-port = strdup(port); -if (!h-port) -goto no_memory; +src = strchr(port, ':'); +if (src) +*src++ = '\0'; + +h-port = strdup(port); +if (!h-port) +goto no_memory; +} if (src STRPREFIX(src, exportname=)) { src = strdup(strchr(src, '=') + 1); @@ -2261,6 +2270,14 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) virBufferEscape(opt, ',', ,, :%s, disk-hosts-port ? disk-hosts-port : 10809); break; +case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX: +if (!disk-hosts-socket) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(socket attribute required for unix transport)); +return -1; +} +virBufferEscape(opt, ',', ,, unix:%s, disk-hosts-socket); +break; default: transp = virDomainDiskProtocolTransportTypeToString(disk-hosts-transport); virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 1731022..12174b3 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -183,6 +183,7 @@ mymain(void) DO_TEST(disk-drive-cache-unsafe); DO_TEST(disk-drive-network-nbd); DO_TEST(disk-drive-network-nbd-export); +DO_TEST(disk-drive-network-nbd-unix); DO_TEST(disk-drive-network-gluster); DO_TEST(disk-drive-network-rbd); DO_TEST(disk-drive-network-rbd-ipv6); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args new file mode 100644 index 000..afea187 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \
[libvirt] [PATCH 12/13] domain: parse XML for iscsi authorization credentials
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/formatdomain.html.in | 12 - docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 31 -- .../qemuxml2argv-disk-drive-network-iscsi-auth.xml | 31 ++ tests/qemuxml2xmltest.c| 1 + 5 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c590427..0906fe9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1760,12 +1760,12 @@ holds the actual password or other credentials (the domain XML intentionally does not expose the password, only the reference to the object that does manage the password). For now, the -only known secret codetype/code is ceph, for Ceph RBD -network sources, and requires either an -attribute codeuuid/code with the UUID of the Ceph secret -object, or an attribute codeusage/code with the name -associated with the Ceph secret -object. span class=sincelibvirt 0.9.7/span +known secret codetype/codes are ceph, for Ceph RBD +network sources, and iscsi, for CHAP authentication of iSCSI +targets. Both require either a codeuuid/code attribute +with the UUID of the secret object, or a codeusage/code +attribute matching the key that was specified in the +secret object. span class=sincelibvirt 0.9.7/span /dd dtcodegeometry/code/dt ddThe optional codegeometry/code element provides the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b8c4503..6f85e84 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3592,6 +3592,7 @@ attribute name='type' choice valueceph/value + valueiscsi/value /choice /attribute choice diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 71da694..e4c3e67 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3885,6 +3885,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *wwn = NULL; char *vendor = NULL; char *product = NULL; +int expected_secret_usage = -1; +int auth_secret_usage = -1; if (VIR_ALLOC(def) 0) { virReportOOMError(); @@ -3922,7 +3924,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (cur-type == XML_ELEMENT_NODE) { if (!source !hosts xmlStrEqual(cur-name, BAD_CAST source)) { - sourceNode = cur; switch (def-type) { @@ -3958,6 +3959,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, _(invalid logical unit number)); goto error; } +expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI; +} else if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { +expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH; } if (!(source = virXMLPropString(cur, name)) def-protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) { @@ -4144,8 +4148,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, _(missing type for secret)); goto error; } -if (virSecretUsageTypeTypeFromString(usageType) != -VIR_SECRET_USAGE_TYPE_CEPH) { +auth_secret_usage = +virSecretUsageTypeTypeFromString(usageType); +if (auth_secret_usage 0) { virReportError(VIR_ERR_XML_ERROR, _(invalid secret type %s), usageType); @@ -4295,6 +4300,13 @@ virDomainDiskDefParseXML(virCapsPtr caps, cur = cur-next; } +if (auth_secret_usage != -1 auth_secret_usage != expected_secret_usage) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(invalid secret type '%s'), + virSecretUsageTypeTypeToString(auth_secret_usage)); +goto error; +} + device = virXMLPropString(node, device); if (device) { if ((def-device = virDomainDiskDeviceTypeFromString(device)) 0) { @@ -12500,15 +12512,18 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def-auth.username) { virBufferEscapeString(buf, auth username='%s'\n, def-auth.username); +if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { +virBufferAsprintf(buf, secret type='iscsi'); +} else
[libvirt] [PATCH 07/13] domain: add support for iscsi network disks
This plumbs in the XML description of iSCSI shares. The next patches will add support for the libiscsi userspace initiator. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + .../qemuxml2argv-disk-drive-network-iscsi.xml | 27 ++ tests/qemuxml2xmltest.c| 1 + 6 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 094b509..fd6d5ae 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1438,7 +1438,7 @@ to the directory to use as the disk. If the disk codetype/code is network, then the codeprotocol/code attribute specifies the protocol to access to the requested image; possible values -are nbd, rbd, sheepdog or gluster. If the +are nbd, iscsi, rbd, sheepdog or gluster. If the codeprotocol/code attribute is rbd, sheepdog or gluster, an additional attribute codename/code is mandatory to specify which volume/image will be used; for nbd it is optional. When the disk @@ -1698,6 +1698,11 @@ td only one /td /tr tr +td iscsi /td +td an iSCSI server /td +td only one /td + /tr + tr td rbd /td td monitor servers of RBD /td td one or more /td diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 63be4aa..980410f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1084,6 +1084,7 @@ valuerbd/value valuesheepdog/value valuegluster/value +valueiscsi/value /choice /attribute optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0c75838..b42c79c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -235,7 +235,8 @@ VIR_ENUM_IMPL(virDomainDiskProtocol, VIR_DOMAIN_DISK_PROTOCOL_LAST, nbd, rbd, sheepdog, - gluster) + gluster, + iscsi) VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST, tcp, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4ffa4aa..7cd0264 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -494,6 +494,7 @@ enum virDomainDiskProtocol { VIR_DOMAIN_DISK_PROTOCOL_RBD, VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG, VIR_DOMAIN_DISK_PROTOCOL_GLUSTER, +VIR_DOMAIN_DISK_PROTOCOL_ISCSI, VIR_DOMAIN_DISK_PROTOCOL_LAST }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml new file mode 100644 index 000..dd85032 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml @@ -0,0 +1,27 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='network' device='disk' + driver name='qemu' type='raw'/ + source protocol='iscsi' name='iqn.1992-01.com.example' +host name='example.org' port='6000'/ + /source + target dev='vda' bus='virtio'/ +/disk +controller type='usb' index='0'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7a01954..e0d3b20 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -171,6 +171,7 @@ mymain(void) DO_TEST(disk-drive-network-nbd-ipv6); DO_TEST(disk-drive-network-nbd-ipv6-export); DO_TEST(disk-drive-network-nbd-unix); +DO_TEST(disk-drive-network-iscsi); DO_TEST(disk-scsi-device); DO_TEST(disk-scsi-vscsi); DO_TEST(disk-scsi-virtio-scsi); -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/13] domain: make port optional for network disks
Only sheepdog actually required it in the code, and we can use 7000 as the default---the same value that QEMU uses for the simple sheepdog:VOLUME syntax. With this change, the schema can be fixed to allow no port. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/formatdomain.html.in | 6 ++ docs/schemas/domaincommon.rng | 8 +--- src/conf/domain_conf.c| 5 - src/qemu/qemu_command.c | 3 ++- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5b4eabe..c590427 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1692,31 +1692,37 @@ th Protocol /th th Meaning /th th Number of hosts /th +th Default port /th /tr tr td nbd /td td a server running nbd-server /td td only one /td +td 10809 /td /tr tr td iscsi /td td an iSCSI server /td td only one /td +td 3260 /td /tr tr td rbd /td td monitor servers of RBD /td td one or more /td +td 6789 /td /tr tr td sheepdog /td td one of the sheepdog servers (default is localhost:7000) /td td zero or one /td +td 7000 /td /tr tr td gluster /td td a server running glusterd daemon /td td only one /td +td 24007 /td /tr /table gluster supports tcp, rdma, unix as valid values for the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 82062a3..b8c4503 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1119,9 +1119,11 @@ ref name=ipAddr/ /choice /attribute -attribute name=port - ref name=unsignedInt/ -/attribute +optional + attribute name=port +ref name=unsignedInt/ + /attribute +/optional /group group attribute name=transport diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b801239..71da694 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4013,11 +4013,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, goto error; } hosts[nhosts - 1].port = virXMLPropString(child, port); -if (!hosts[nhosts - 1].port) { -virReportError(VIR_ERR_XML_ERROR, - %s, _(missing port for host)); -goto error; -} } } child = child-next; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 59773ec..5d52be5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2526,7 +2526,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } else { /* only one host is supported now */ virBufferAsprintf(opt, file=sheepdog:%s:%s:, - disk-hosts-name, disk-hosts-port); + disk-hosts-name, + disk-hosts-port ? disk-hosts-port : 7000); virBufferEscape(opt, ',', ,, %s,, disk-src); } break; -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/13] qemu: pass iscsi authorization credentials
A better way to do this would be to use a configuration file like [iscsi target-name] user = name password = pwd and pass it via -readconfig. This would remove the username and password from the ps output. For now, however, keep this solution. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_command.c| 80 ++ ...qemuxml2argv-disk-drive-network-iscsi-auth.args | 1 + tests/qemuxml2argvtest.c | 2 + 3 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d52be5..30ddbd3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1888,8 +1888,8 @@ qemuBuildRBDString(virConnectPtr conn, VIR_FREE(base64); } else { virReportError(VIR_ERR_INTERNAL_ERROR, - _(rbd username '%s' specified but secret not found), - disk-auth.username); + _(%s username '%s' specified but secret not found), + rbd, disk-auth.username); goto error; } } else { @@ -2057,6 +2057,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, char *transp = NULL; char *sock = NULL; char *volimg = NULL; +char *secret = NULL; if (VIR_ALLOC(def-hosts) 0) goto no_memory; @@ -2117,6 +2118,16 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, def-src = NULL; } +if (uri-user) { +secret = strchr(uri-user, ':'); +if (secret) +*secret = '\0'; + +def-auth.username = strdup(uri-user); +if (!def-auth.username) +goto no_memory; +} + def-nhosts = 1; ret = 0; @@ -2237,14 +2248,20 @@ error: } static int -qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt, -const char *scheme) +qemuBuildDriveURIString(virConnectPtr conn, +virDomainDiskDefPtr disk, virBufferPtr opt, +const char *scheme, virSecretUsageType secretType) { int ret = -1; int port = 0; +virSecretPtr sec = NULL; +char *secret = NULL; +size_t secret_size; + char *tmpscheme = NULL; char *volimg = NULL; char *sock = NULL; +char *user = NULL; char *builturi = NULL; const char *transp = NULL; virURI uri = { @@ -2280,8 +2297,42 @@ qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt, virAsprintf(sock, socket=%s, disk-hosts-socket) 0) goto no_memory; +if (disk-auth.username secretType != VIR_SECRET_USAGE_TYPE_NONE) { +/* look up secret */ +switch (disk-auth.secretType) { +case VIR_DOMAIN_DISK_SECRET_TYPE_UUID: +sec = virSecretLookupByUUID(conn, +disk-auth.secret.uuid); +break; +case VIR_DOMAIN_DISK_SECRET_TYPE_USAGE: +sec = virSecretLookupByUsage(conn, secretType, + disk-auth.secret.usage); +break; +} + +if (sec) { +secret = (char *)conn-secretDriver-getValue(sec, secret_size, 0, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); +if (secret == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(could not get the value of the secret for username %s), + disk-auth.username); +ret = -1; +goto cleanup; +} +if (virAsprintf(user, %s:%s, disk-auth.username, secret) 0) +goto no_memory; +} else { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(%s username '%s' specified but secret not found), + scheme, disk-auth.username); +ret = -1; +goto cleanup; +} +} uri.scheme = tmpscheme; /* gluster+transport */ uri.server = disk-hosts-name; +uri.user = user; uri.port = port; uri.path = volimg; uri.query = sock; @@ -2305,16 +2356,18 @@ no_memory: } static int -qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +qemuBuildGlusterString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) { -return qemuBuildDriveURIString(disk, opt, gluster); +return qemuBuildDriveURIString(conn, disk, opt, gluster, + VIR_SECRET_USAGE_TYPE_NONE); } static int -qemuBuildISCSIString(virDomainDiskDefPtr disk, virBufferPtr opt) +qemuBuildISCSIString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) { int ret; -ret = qemuBuildDriveURIString(disk, opt,
Re: [libvirt] [PATCH] Fix crash changing CDROM media
On 02/25/2013 10:25 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com This change tried to fix a crash with changing CDROM media but failed to actually do so commit d0172d2b1b5d865aaa042070d7c2d00effb2ff8c Author: Osier Yang jy...@redhat.com Date: Tue Feb 19 20:27:45 2013 +0800 qemu: Remove the shared disk entry if the operation is ejecting or updating It was still accessing disk-src, when the entire 'disk' object has been free'd already. Even if it weren't free'd, accessing the 'src' value of virDomainDiskDef is not allowed without first validating disk-type is file or block. Just remove the broken code entirely. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_driver.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) ACK. -- Eric Blake eblake 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 00/13] Network disk improvements (NBD libiscsi)
This series improves support for NBD disks (patches 1-6), and adds support for the libiscsi userspace initiator (patches 7-13). Please review! Paolo Paolo Bonzini (13): qemu: fix use-after-free when parsing NBD disk qemu: do not support non-network disks without -drive qemu: rewrite NBD command-line builder and parser qemu: support named nbd exports qemu: support NBD with Unix sockets qemu: support URI syntax for NBD domain: add support for iscsi network disks qemu: add support for libiscsi qemu: support LUN numbers for iSCSI disks domain: make port optional for network disks secret: add iscsi to possible usage types domain: parse XML for iscsi authorization credentials qemu: pass iscsi authorization credentials docs/formatdomain.html.in | 42 +- docs/formatsecret.html.in | 12 + docs/schemas/domaincommon.rng | 37 +- docs/schemas/secret.rng| 10 + include/libvirt/libvirt.h.in | 1 + src/conf/domain_conf.c | 51 ++- src/conf/domain_conf.h | 3 + src/conf/secret_conf.c | 22 +- src/conf/secret_conf.h | 1 + src/qemu/qemu_command.c| 432 ++--- src/secret/secret_driver.c | 8 + tests/qemuargv2xmltest.c | 5 + .../qemuxml2argv-disk-drive-network-gluster.args | 2 +- ...qemuxml2argv-disk-drive-network-iscsi-auth.args | 1 + .../qemuxml2argv-disk-drive-network-iscsi-auth.xml | 31 ++ .../qemuxml2argv-disk-drive-network-iscsi.args | 1 + .../qemuxml2argv-disk-drive-network-iscsi.xml | 34 ++ ...qemuxml2argv-disk-drive-network-nbd-export.args | 5 + .../qemuxml2argv-disk-drive-network-nbd-export.xml | 33 ++ ...ml2argv-disk-drive-network-nbd-ipv6-export.args | 5 + ...xml2argv-disk-drive-network-nbd-ipv6-export.xml | 33 ++ .../qemuxml2argv-disk-drive-network-nbd-ipv6.args | 5 + .../qemuxml2argv-disk-drive-network-nbd-ipv6.xml | 33 ++ .../qemuxml2argv-disk-drive-network-nbd-unix.args | 5 + .../qemuxml2argv-disk-drive-network-nbd-unix.xml | 33 ++ tests/qemuxml2argvtest.c | 12 + tests/qemuxml2xmltest.c| 7 + 27 files changed, 687 insertions(+), 177 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/13] qemu: support LUN numbers for iSCSI disks
Each iSCSI target can provide multiple logical units. Support this with an additional attribute in the source element. libiscsi places the LUN in the path component of the URI. iSCSI names cannot contain slashes, so this is unambiguous Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/formatdomain.html.in | 9 --- docs/schemas/domaincommon.rng | 29 +++--- src/conf/domain_conf.c | 12 + src/conf/domain_conf.h | 2 ++ src/qemu/qemu_command.c| 23 + .../qemuxml2argv-disk-drive-network-iscsi.args | 2 +- .../qemuxml2argv-disk-drive-network-iscsi.xml | 7 ++ 7 files changed, 65 insertions(+), 19 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fd6d5ae..5b4eabe 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1441,10 +1441,11 @@ are nbd, iscsi, rbd, sheepdog or gluster. If the codeprotocol/code attribute is rbd, sheepdog or gluster, an additional attribute codename/code is mandatory to specify which -volume/image will be used; for nbd it is optional. When the disk -codetype/code is network, the codesource/code may have zero -or more codehost/code sub-elements used to specify the hosts -to connect. +volume/image will be used; for nbd it is optional. iscsi supports +an additional numeric attribute codeunit/code for the logical unit +number (default 0). When the disk codetype/code is network, +the codesource/code may have zero or more codehost/code +sub-elements used to specify the hosts to connect. span class=sinceSince 0.0.3; codetype='dir'/code since 0.7.5; codetype='network'/code since 0.8.7/spanbr/ For a file disk type which represents a cdrom or floppy diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 980410f..82062a3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1078,15 +1078,26 @@ interleave optional element name=source -attribute name=protocol - choice -valuenbd/value -valuerbd/value -valuesheepdog/value -valuegluster/value -valueiscsi/value - /choice -/attribute +choice + attribute name=protocol +choice + valuenbd/value + valuerbd/value + valuesheepdog/value + valuegluster/value +/choice + /attribute + group +attribute name=protocol + valueiscsi/value +/attribute +optional + attribute name=unit +data type=integer/ + /attribute +/optional + /group +/choice optional attribute name=name/ /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b42c79c..b801239 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3950,6 +3950,15 @@ virDomainDiskDefParseXML(virCapsPtr caps, protocol); goto error; } +if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { +if (virXMLPropString(cur, unit) +virXPathUInt(string(./source/@unit), + ctxt, def-unit) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(invalid logical unit number)); +goto error; +} +} if (!(source = virXMLPropString(cur, name)) def-protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -12555,6 +12564,9 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def-src) { virBufferEscapeString(buf, name='%s', def-src); } +if (def-protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI def-unit) { +virBufferAsprintf(buf, unit='%u', def-unit); +} if (def-nhosts == 0) { virBufferAddLit(buf, /\n); } else { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7cd0264..ac8a446 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -606,6 +606,8 @@ struct
[libvirt] [PATCH 01/13] qemu: fix use-after-free when parsing NBD disk
disk-src is still used for disks-hosts-name, do not free it. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_command.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dee493f..5dccaae 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8707,7 +8707,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, disk-hosts-port = strdup(port); if (!disk-hosts-port) goto no_memory; -VIR_FREE(disk-src); disk-src = NULL; break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/13] qemu: support URI syntax for NBD
QEMU 1.3 and newer support an alternative URI-based syntax to specify the location of an NBD server. Libvirt can keep on using the old syntax in general, but only the URI syntax supports IPv6 addresses. The URI syntax also supports relative paths to Unix sockets. These should never be used but aren't explicitly blocked either by the parser, so support it just in case. The URI syntax is intentionally compatible with Gluster's, and the code can be reused. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_command.c| 97 +++--- tests/qemuargv2xmltest.c | 2 + ...ml2argv-disk-drive-network-nbd-ipv6-export.args | 5 ++ ...xml2argv-disk-drive-network-nbd-ipv6-export.xml | 33 .../qemuxml2argv-disk-drive-network-nbd-ipv6.args | 5 ++ .../qemuxml2argv-disk-drive-network-nbd-ipv6.xml | 33 tests/qemuxml2argvtest.c | 4 + tests/qemuxml2xmltest.c| 2 + 8 files changed, 153 insertions(+), 28 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89cd065..733d3bf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2050,35 +2050,36 @@ no_memory: } static int -qemuParseGlusterString(virDomainDiskDefPtr def) +qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, +const char *scheme) { int ret = -1; char *transp = NULL; char *sock = NULL; char *volimg = NULL; -virURIPtr uri = NULL; - -if (!(uri = virURIParse(def-src))) { -return -1; -} if (VIR_ALLOC(def-hosts) 0) goto no_memory; -if (STREQ(uri-scheme, gluster)) { +transp = strchr(uri-scheme, '+'); +if (transp) +*transp++ = 0; + +if (!STREQ(uri-scheme, scheme)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Invalid transport/scheme '%s'), uri-scheme); +goto error; +} + +if (!transp) { def-hosts-transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; -} else if (STRPREFIX(uri-scheme, gluster+)) { -transp = strchr(uri-scheme, '+') + 1; +} else { def-hosts-transport = virDomainDiskProtocolTransportTypeFromString(transp); if (def-hosts-transport 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _(Invalid gluster transport type '%s'), transp); + _(Invalid %s transport type '%s'), scheme, transp); goto error; } -} else { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(Invalid transport/scheme '%s'), uri-scheme); -goto error; } def-nhosts = 0; /* set to 1 once everything succeeds */ @@ -2105,11 +2106,16 @@ qemuParseGlusterString(virDomainDiskDefPtr def) } } } -volimg = uri-path + 1; /* skip the prefix slash */ -VIR_FREE(def-src); -def-src = strdup(volimg); -if (!def-src) -goto no_memory; +if (uri-path) { +volimg = uri-path + 1; /* skip the prefix slash */ +VIR_FREE(def-src); +def-src = strdup(volimg); +if (!def-src) +goto no_memory; +} else { +VIR_FREE(def-src); +def-src = NULL; +} def-nhosts = 1; ret = 0; @@ -2128,12 +2134,31 @@ error: } static int +qemuParseGlusterString(virDomainDiskDefPtr def) +{ +virURIPtr uri = NULL; + +if (!(uri = virURIParse(def-src))) +return -1; + +return qemuParseDriveURIString(def, uri, gluster); +} + +static int qemuParseNBDString(virDomainDiskDefPtr disk) { virDomainDiskHostDefPtr h = NULL; char *host, *port; char *src; +virURIPtr uri = NULL; + +if (strstr(disk-src, ://)) { +uri = virURIParse(disk-src); +if (uri) +return qemuParseDriveURIString(disk, uri, nbd); +} + if (VIR_ALLOC(h) 0) goto no_memory; @@ -2190,7 +2215,8 @@ error: } static int -qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt, +const char *scheme) { int ret = -1; int port = 0; @@ -2204,18 +2230,18 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) }; if (disk-nhosts != 1) { -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(gluster accepts only one host)); +virReportError(VIR_ERR_INTERNAL_ERROR, + _(%s accepts only one host), scheme);
[libvirt] [PATCH] qemu: Don't fail to shutdown domains with unresponsive agent
Currently, qemuDomainShutdownFlags() chooses the agent method of shutdown whenever the agent is configured. However, this assumption is not enough as the guest agent may be unresponsive at the moment. So unless guest agent method has been explicitly requested, we should fall back to the ACPI method. --- src/qemu/qemu_driver.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 435c37c..06b14ae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1702,7 +1702,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; -bool useAgent = false; +bool useAgent = false, agentRequested; virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1); @@ -1719,23 +1719,32 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { goto cleanup; priv = vm-privateData; +agentRequested = flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT; -if ((flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || +if (agentRequested || (!(flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) priv-agent)) useAgent = true; if (useAgent) { if (priv-agentError) { -virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s, - _(QEMU guest agent is not - available due to an error)); -goto cleanup; +if (agentRequested) { +virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s, + _(QEMU guest agent is not + available due to an error)); +goto cleanup; +} else { +useAgent = false; +} } if (!priv-agent) { -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, - _(QEMU guest agent is not configured)); -goto cleanup; +if (agentRequested) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(QEMU guest agent is not configured)); +goto cleanup; +} else { +useAgent = false; +} } } @@ -1752,7 +1761,12 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { qemuDomainObjEnterAgent(vm); ret = qemuAgentShutdown(priv-agent, QEMU_AGENT_SHUTDOWN_POWERDOWN); qemuDomainObjExitAgent(vm); -} else { +} + +/* If we are not enforced to use just an agent, try ACPI + * shutdown as well in case agent did not succeed */ +if (!useAgent || +((ret 0) !agentRequested)) { qemuDomainSetFakeReboot(driver, vm, false); qemuDomainObjEnterMonitor(driver, vm); -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/13] qemu: rewrite NBD command-line builder and parser
Move the code to an external function, and structure it to prepare the addition of new features in the next few patches. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_command.c | 128 tests/qemuxml2xmltest.c | 1 + 2 files changed, 76 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a3c5a4e..beb7cfe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2128,6 +2128,45 @@ error: } static int +qemuParseNBDString(virDomainDiskDefPtr disk) +{ +virDomainDiskHostDefPtr h = NULL; +char *host, *port; + +if (VIR_ALLOC(h) 0) +goto no_memory; + +host = disk-src + strlen(nbd:); +port = strchr(host, ':'); +if (!port) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(cannot parse nbd filename '%s'), disk-src); +goto error; +} + +*port++ = '\0'; +h-name = strdup(host); +if (!h-name) +goto no_memory; + +h-port = strdup(port); +if (!h-port) +goto no_memory; + +VIR_FREE(disk-src); +disk-nhosts = 1; +disk-hosts = h; +return 0; + +no_memory: +virReportOOMError(); +error: +virDomainDiskHostDefFree(h); +VIR_FREE(h); +return -1; +} + +static int qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) { int ret = -1; @@ -2188,6 +2227,36 @@ no_memory: goto cleanup; } +static int +qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ +const char *transp; + +if (disk-nhosts != 1) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(nbd accepts only one host)); +return -1; +} + +virBufferAddLit(opt, file=nbd:); + +switch (disk-hosts-transport) { +case VIR_DOMAIN_DISK_PROTO_TRANS_TCP: +if (disk-hosts-name) +virBufferEscape(opt, ',', ,, %s, disk-hosts-name); +virBufferEscape(opt, ',', ,, :%s, +disk-hosts-port ? disk-hosts-port : 10809); +break; +default: +transp = virDomainDiskProtocolTransportTypeToString(disk-hosts-transport); +virReportError(VIR_ERR_INTERNAL_ERROR, + _(nbd does not support transport '%s'), transp); +break; +} + +return 0; +} + char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, @@ -2314,13 +2383,9 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk-protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: -if (disk-nhosts != 1) { -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(NBD accepts only one host)); +if (qemuBuildNBDString(disk, opt) 0) goto error; -} -virBufferAsprintf(opt, file=nbd:%s:%s,, - disk-hosts-name, disk-hosts-port); +virBufferAddChar(opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: virBufferAddLit(opt, file=); @@ -7337,39 +7402,11 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps, if (STRPREFIX(def-src, /dev/)) def-type = VIR_DOMAIN_DISK_TYPE_BLOCK; else if (STRPREFIX(def-src, nbd:)) { -char *host, *port; - def-type = VIR_DOMAIN_DISK_TYPE_NETWORK; def-protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; -host = def-src + strlen(nbd:); -port = strchr(host, ':'); -if (!port) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(cannot parse nbd filename '%s'), - def-src); -goto error; -} -*port++ = '\0'; -if (VIR_ALLOC(def-hosts) 0) { -virReportOOMError(); -goto error; -} -def-nhosts = 1; -def-hosts-name = strdup(host); -if (!def-hosts-name) { -virReportOOMError(); -goto error; -} -def-hosts-port = strdup(port); -if (!def-hosts-port) { -virReportOOMError(); -goto error; -} -def-hosts-transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; -def-hosts-socket = NULL; -VIR_FREE(def-src); -def-src = NULL; +if (qemuParseNBDString(def) 0) +goto error; } else if
[libvirt] [PATCH 11/13] secret: add iscsi to possible usage types
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- docs/formatsecret.html.in| 12 docs/schemas/secret.rng | 10 ++ include/libvirt/libvirt.h.in | 1 + src/conf/secret_conf.c | 22 +- src/conf/secret_conf.h | 1 + src/secret/secret_driver.c | 8 6 files changed, 53 insertions(+), 1 deletion(-) diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 01aff2d..c3c4a25 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -66,6 +66,18 @@ device/a. span class=sinceSince 0.9.7/span. /p +h3Usage type iscsi/h3 + +p + This secret is associated with an iSCSI target for CHAP authentication. + The codelt;usage type='iscsi'gt;/code element must contain + a single codetarget/code element that specifies a usage name + for the secret. The iSCSI secret can then be used by UUID or by + this usage name via the codelt;authgt;/code element of + a a href=domain.html#elementsDisksdisk + device/a. span class=sinceSince 1.0.4/span. +/p + h2a name=exampleExample/a/h2 pre diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index e49bd5a..d7b8f83 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -41,6 +41,7 @@ choice ref name='usagevolume'/ ref name='usageceph'/ + ref name='usageiscsi'/ !-- More choices later -- /choice /element @@ -67,4 +68,13 @@ /element /define + define name='usageiscsi' +attribute name='type' + valueiscsi/value +/attribute +element name='target' + ref name='genericName'/ +/element + /define + /grammar diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad30cd0..3f98ebe 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3641,6 +3641,7 @@ typedef enum { VIR_SECRET_USAGE_TYPE_NONE = 0, VIR_SECRET_USAGE_TYPE_VOLUME = 1, VIR_SECRET_USAGE_TYPE_CEPH = 2, +VIR_SECRET_USAGE_TYPE_ISCSI = 3, #ifdef VIR_ENUM_SENTINELS VIR_SECRET_USAGE_TYPE_LAST diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 891af65..06b9bb2 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -36,7 +36,7 @@ #define VIR_FROM_THIS VIR_FROM_SECRET VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST, - none, volume, ceph) + none, volume, ceph, iscsi) void virSecretDefFree(virSecretDefPtr def) @@ -57,6 +57,10 @@ virSecretDefFree(virSecretDefPtr def) VIR_FREE(def-usage.ceph); break; +case VIR_SECRET_USAGE_TYPE_ISCSI: +VIR_FREE(def-usage.target); +break; + default: VIR_ERROR(_(unexpected secret usage type %d), def-usage_type); break; @@ -108,6 +112,15 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, } break; +case VIR_SECRET_USAGE_TYPE_ISCSI: +def-usage.target = virXPathString(string(./usage/target), ctxt); +if (!def-usage.target) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Ceph usage specified, but target is missing)); +return -1; +} +break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _(unexpected secret usage type %d), @@ -262,6 +275,13 @@ virSecretDefFormatUsage(virBufferPtr buf, } break; +case VIR_SECRET_USAGE_TYPE_ISCSI: +if (def-usage.target != NULL) { +virBufferEscapeString(buf, target%s/target\n, + def-usage.target); +} +break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _(unexpected secret usage type %d), diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index 6079d5b..53517f9 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -39,6 +39,7 @@ struct _virSecretDef { union { char *volume; /* May be NULL */ char *ceph; +char *target; } usage; }; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 5be33b9..c577817 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -149,6 +149,11 @@ secretFindByUsage(virSecretDriverStatePtr driver, int usageType, const char *usa if (STREQ(s-def-usage.ceph, usageID)) return s; break; + +case VIR_SECRET_USAGE_TYPE_ISCSI: +if (STREQ(s-def-usage.target, usageID)) +return s; +break; } } return NULL; @@ -614,6 +619,9 @@ secretUsageIDForDef(virSecretDefPtr def) case VIR_SECRET_USAGE_TYPE_CEPH: return def-usage.ceph; +case VIR_SECRET_USAGE_TYPE_ISCSI: +return def-usage.target; + default: return
[libvirt] [PATCH 14/13] qemu: support passthrough for iscsi disks
This enables usage of commands like persistent reservations. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c| 9 ++- .../qemuxml2argv-disk-drive-network-iscsi-lun.args | 1 + .../qemuxml2argv-disk-drive-network-iscsi-lun.xml | 28 ++ tests/qemuxml2argvtest.c | 4 5 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f399871..8acff1a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -156,6 +156,7 @@ virDomainDiskIoTypeToString; virDomainDiskPathByName; virDomainDiskProtocolTransportTypeFromString; virDomainDiskProtocolTransportTypeToString; +virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; virDomainDiskTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 30ddbd3..22e9d81 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2844,7 +2844,14 @@ qemuBuildDriveDevStr(virDomainDefPtr def, bus); goto error; } -if (disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK) { +if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK) { +if (disk-protocol != VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(disk device='lun' is not supported for protocol='%s'), + virDomainDiskProtocolTypeToString(disk-protocol)); +goto error; +} +} else if (disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(disk device='lun' is not supported for type='%s'), virDomainDiskTypeToString(disk-type)); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args new file mode 100644 index 000..baa7760 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb -drive file=iscsi://example.org:3260/iqn.1992-01.com.example,if=none,id=drive-scsi0-0-0-0,format=raw -device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml new file mode 100644 index 000..b6d6aaa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml @@ -0,0 +1,28 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='network' device='lun' + driver name='qemu' type='raw'/ + source protocol='iscsi' name='iqn.1992-01.com.example' +host name='example.org' port='3260'/ + /source + target dev='sda' bus='scsi'/ +/disk +controller type='usb' index='0'/ +controller type='scsi' model='virtio-scsi'/ +memballoon model='none'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0afecf3..5e0d976 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -497,6 +497,10 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST(disk-drive-network-iscsi-auth, QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); +DO_TEST(disk-drive-network-iscsi-lun, +QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE_FORMAT, +QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI_PCI, +QEMU_CAPS_VIRTIO_BLK_SG_IO, QEMU_CAPS_SCSI_BLOCK); DO_TEST(disk-drive-network-gluster, QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST(disk-drive-network-rbd, -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] network: add force attribute for dhcp options
If a dhcp option has force='yes', the dhcp server will send that option back to every client regardless of whether or not the client requests that option (without force='yes', an option will only be sent to those clients that ask for it in their request packet). For example: option number='40' value='libvirt' force='yes'/ This information is relayed to dnsmasq by using the dhcp-option-force option, rather than dhcp-option. --- Changes from V1: * add forgotten addition to RNG * add forgotten addition to virNetworkIpDefFormat * add networkxml2xml test (which would have caught the above) docs/formatnetwork.html.in | 21 + docs/schemas/network.rng | 8 src/conf/network_conf.c| 16 src/conf/network_conf.h| 3 ++- src/network/bridge_driver.c| 3 ++- tests/networkxml2confdata/nat-network.conf | 2 +- tests/networkxml2confdata/nat-network.xml | 2 +- tests/networkxml2xmlin/nat-network.xml | 5 + tests/networkxml2xmlout/nat-network.xml| 5 + 9 files changed, 61 insertions(+), 4 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 41a83fa..97e3bd4 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -557,6 +557,10 @@ lt;range start=192.168.122.100 end=192.168.122.254 /gt; lt;host mac=00:16:3e:77:e2:ed name=foo.example.com ip=192.168.122.10 /gt; lt;host mac=00:16:3e:3e:a9:1a name=bar.example.com ip=192.168.122.11 /gt; +lt;option number='252' value='\n'/gt; +lt;option number='4' value='192.168.122.11'/gt; +lt;option number='6' value='192.168.145.23'/gt; +lt;option number='23' value='64' force='yes'/gt; lt;/dhcpgt; lt;/ipgt; lt;ip family=ipv6 address=2001:db8:ca2:2::1 prefix=64 /gt; @@ -699,6 +703,23 @@ for all address ranges and statically assigned addresses.span class=sinceSince 0.7.1 (codeserver/code since 0.7.3)./span /dd + dtcodeoption/code/dt + ddThe optional codeoption/code element (which can +be repeated for multiple DHCP options) specifies +generic DHCP options (as defined in RFC 2132, or later +RFCs in some cases) that will be sent by the DHCP +server to requesting clients (IPv4 only). There are +three possible attributes: codenumber/code is +mandatory and gives the standard option +number; codevalue/code is optional and gives the +value of that option to be provided to the client +codeforce/code is also optional, and defaults to +no; if codeforce/code is yes, the option will +be sent to all DHCP clients regardless of whether or +not the client requests it (usually only options +specifically requested by the client are sent in the +DHCP response).span class=sinceSince 1.0.3/span + /dd /dl /dd /dl diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index da7d8ad..ed6964a 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -305,6 +305,14 @@ element name=option attribute name=numberref name=uint8range//attribute attribute name=valuetext//attribute +optional + attribute name=force +choice + valueyes/value + valueno/value +/choice + /attribute +/optional /element /zeroOrMore /element diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 8b57242..4d07bac 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -796,6 +796,7 @@ virNetworkDHCPOptionDefParseXML(const char *networkName, virNetworkDHCPOptionDefPtr option) { char *number = NULL; +char *force = NULL; int ret = -1; if (!(number = virXMLPropString(node, number))) { @@ -811,12 +812,25 @@ virNetworkDHCPOptionDefParseXML(const char *networkName, number); goto cleanup; } + option-value = virXMLPropString(node, value); +if ((force = virXMLPropString(node, force))) { + if (STRCASEEQ(force, yes)) { + option-force = true; + } else if (STRCASENEQ(force, no)) { + virReportError(VIR_ERR_XML_ERROR, + _(Invalid option force attribute %s +in network '%s'), force, networkName); + goto cleanup; + } +} + ret = 0; cleanup:
Re: [libvirt] [PATCH] qemu: Don't fail to shutdown domains with unresponsive agent
On 02/25/2013 10:55 AM, Michal Privoznik wrote: Currently, qemuDomainShutdownFlags() chooses the agent method of shutdown whenever the agent is configured. However, this assumption is not enough as the guest agent may be unresponsive at the moment. So unless guest agent method has been explicitly requested, we should fall back to the ACPI method. --- src/qemu/qemu_driver.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) When Daniel added two new shutdown methods for LXC, I had the question on whether shutdown methods should be mutually exclusive, or a bitmask of all permissible things to attempt (where passing 0 leaves the attempts up the hypervisor). The consensus was that allowing the user to pass more than one method makes sense (although the hypervisor then gets to choose method priorities). I'm not sure your logic matches the goal of allowing the user to request both agent and acpi at the same time. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 435c37c..06b14ae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1702,7 +1702,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; -bool useAgent = false; +bool useAgent = false, agentRequested; virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN | VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1); @@ -1719,23 +1719,32 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { goto cleanup; priv = vm-privateData; +agentRequested = flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT; -if ((flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) || +if (agentRequested || (!(flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) priv-agent)) useAgent = true; A better logic might be: /* Explicitly requested agent, or no requests made and agent seems possible */ if (agentRequested || (!flags priv-agent)) if (useAgent) { if (priv-agentError) { -virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s, - _(QEMU guest agent is not - available due to an error)); -goto cleanup; +if (agentRequested) { +virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s, + _(QEMU guest agent is not + available due to an error)); +goto cleanup; +} else { +useAgent = false; +} } Here, if there is an agent error, but the user requested both agent and acpi flags at the same time, then you want to fall back to acpi instead of erroring out completely. if (!priv-agent) { -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, - _(QEMU guest agent is not configured)); -goto cleanup; +if (agentRequested) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(QEMU guest agent is not configured)); +goto cleanup; +} else { +useAgent = false; +} } } @@ -1752,7 +1761,12 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { qemuDomainObjEnterAgent(vm); ret = qemuAgentShutdown(priv-agent, QEMU_AGENT_SHUTDOWN_POWERDOWN); qemuDomainObjExitAgent(vm); -} else { +} + +/* If we are not enforced to use just an agent, try ACPI + * shutdown as well in case agent did not succeed */ +if (!useAgent || +((ret 0) !agentRequested)) { Redundant () around ret0. Here, the fallback seems like it should be: /* Get here if agent request failed or was not attempted. Try acpi unless it was excluded from an explicit request */ if (ret 0 (!flags || (flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN))) { -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] storage: fix uid_t|gid_t handling on 32 bit Linux
On 02/22/2013 09:55 AM, Philipp Hahn wrote: uid_t and gid_t are opaque types, ranging from s32 to u32 to u64. Unfortunately libvirt uses the value -1 to mean current process, which on Linux32 gets converted to ALLONE := +(2^32-1) = 4294967295. I like the standardized name INT_MAX better than the invented name ALLONE. Different libvirt versions used different formatting in the past, which break one or the other parsing function: virXPathLong(): (signed long)-1 != (double)ALLONE virXPathULong(): (unsigned long)-1 != (double)-1 Roll our own version of virXPathULong(), which also accepts -1, which is silently converted to ALLONE. For output: do the reverse and print -1 instead of ALLONE. Signed-off-by: Philipp Hahn h...@univention.de --- src/conf/storage_conf.c | 74 ++- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9134a22..b267f00 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -665,7 +665,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, const char *permxpath, int defaultmode) { char *mode; -long v; +double d; Eww - why do we need to use a double? Just parse to a 32-bit int, then check for truncation after the fact (as I said elsewhere in the series, we already do a compile-time verify that uid_t is not larger than int; so the real problem is if uid_t is smaller than int). int ret = -1; xmlNodePtr relnode; xmlNodePtr node; @@ -699,26 +699,57 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, VIR_FREE(mode); } +/* uid_t and gid_t are *opaque* types: on Linux they're u32, on Windows64 + * they're u64, on Solaris they were s32 in the past. There may be others. Not accurate. mingw64 has s64 pid_t (not u64); and completely lacks uid_t and gid_t natively: $ printf '#include sys/types.h\nuid_t\n' | \ x86_64-w64-mingw32-gcc -E -|grep id_t typedef long long _pid_t; typedef _pid_t pid_t; uid_t When using gnulib (as libvirt does), gnulib provides [ug]id_t as 32-bit types even on mingw64. Furthermore, a lot of the problems stem from the fact that s16 or u16 uid_t/gid_t have been used in the past (u16 promotes to s32 without sign extension, so it does not compare equal to (s32)-1). I like the idea of outputting -1 rather than the unsigned all-ones value; which means you need to respin this patch to avoid testsuite failures where we change the test output. Also, anywhere that the parser doesn't accept -1, we need to fix that; accepting -1 as the only valid negative value and requiring all other values to be non-negative makes sense. Looking forward to v2; this patch is appropriate to get into the 1.0.3 release if we get it respun in time. -- Eric Blake eblake 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 tck] Avoid multicast MAC addrs in tests
On 02/25/2013 11:52 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Certain MAC addrs are used for multicast and recent libvirt will refuse to start VMs using those addrs. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- scripts/domain/215-nic-hotplug-many.t | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/domain/215-nic-hotplug-many.t b/scripts/domain/215-nic-hotplug-many.t index 5b935cc..8b4afbd 100644 --- a/scripts/domain/215-nic-hotplug-many.t +++ b/scripts/domain/215-nic-hotplug-many.t @@ -47,9 +47,9 @@ diag Creating a new transient domain; my $dom; ok_domain(sub { $dom = $conn-create_domain($xml) }, created transient domain object); -my $mac1 = 01:11:22:33:44:55; -my $mac2 = 02:11:22:33:44:55; -my $mac3 = 03:11:22:33:44:55; +my $mac1 = 02:11:22:33:44:51; +my $mac2 = 02:11:22:33:44:52; +my $mac3 = 02:11:22:33:44:53; my $model = virtio; my $netxml1 = EOF; ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] uid_t|gid_t fixes for 32 bit Linux
On 02/22/2013 09:58 AM, Philipp Hahn wrote: @Guido: for Debian only the 4th patch is relevant, which applies with some re-indention. Unfortunately, the 4th patch is the one that needs rework. Additional issues: - UID and GID 4294967295 happens to be the same as -1, so hopefully nobody uses it. Should this be documented somewhere? Unless someone ever invents a system with 64-bit uid_t (to date, no one has - your claims about u64 to the contrary), treating 4294967295 as (uid_t)-1 is safe. POSIX already guaranteed that (uid_t)-1 cannot be used as a valid user id. - Some regression test? Did you run 'make check' on your patch? We already have XML output that is affected by the change in output format, and can write the test so that we prove that multiple ways of formatting input all get canonicalized to the same output. -- Eric Blake eblake 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] [PATCHv2] network: add force attribute for dhcp options
On 02/25/2013 11:30 AM, Laine Stump wrote: If a dhcp option has force='yes', the dhcp server will send that option back to every client regardless of whether or not the client requests that option (without force='yes', an option will only be sent to those clients that ask for it in their request packet). For example: option number='40' value='libvirt' force='yes'/ This information is relayed to dnsmasq by using the dhcp-option-force option, rather than dhcp-option. --- Changes from V1: * add forgotten addition to RNG * add forgotten addition to virNetworkIpDefFormat * add networkxml2xml test (which would have caught the above) I'm sure there are useful symbolic names for the various options, instead of just number='40'; but like you said, that can be added later as needed. It's borderline on whether to take this patch without support for option names, since it was submitted prior to freeze but not reviewed until after. It is cleaning up the feature of option parsing that was just barely applied prior to freeze, so that argues that it can be classed as rounding out an incomplete feature. Delaying until post-release would let you get option names in the same release, but then we might want to revert the option support that just went in. Meanwhile, since both this patch, and any future patch for adding option name support, deals with just XML additions, they can easily be backported by distros without a soname bump, regardless of whether they make this release or not; and since it is just XML additions, I don't see how it could cause regressions to existing domain XML. I'm probably 60:40 in favor of including this patch now, but if anyone else speaks clearly for or against taking it in 1.0.3, I can be swayed. + dtcodeoption/code/dt + ddThe optional codeoption/code element (which can +be repeated for multiple DHCP options) specifies +generic DHCP options (as defined in RFC 2132, or later +RFCs in some cases) that will be sent by the DHCP +server to requesting clients (IPv4 only). There are +three possible attributes: codenumber/code is +mandatory and gives the standard option +number; codevalue/code is optional and gives the +value of that option to be provided to the client +codeforce/code is also optional, and defaults to +no; if codeforce/code is yes, the option will +be sent to all DHCP clients regardless of whether or +not the client requests it (usually only options +specifically requested by the client are sent in the +DHCP response).span class=sinceSince 1.0.3/span We definitely want the docs for option, even if we defer the force= attribute until post-release. @@ -811,12 +812,25 @@ virNetworkDHCPOptionDefParseXML(const char *networkName, number); goto cleanup; } + option-value = virXMLPropString(node, value); +if ((force = virXMLPropString(node, force))) { + if (STRCASEEQ(force, yes)) { I don't know that we have done case-insensitive matching in many other places; but being liberal in what we accept doesn't hurt my feelings. On the other hand, the RNG rejects any uppercase variant. ACK. -- Eric Blake eblake 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] [Qemu-devel] [PATCH] vl.c: Support multiple CPU ranges on -numa option
On 02/21/2013 01:57 PM, Eduardo Habkost wrote: Note that the following format, currently used by libvirt: -numa nodes,cpus=A,B,C,D will _not_ work yet, as , is the option separator for the command-line option parser, and it will require changing the -numa option parsing code to handle cpus as a special case. No way. Agreed. :-) The bad news is that libvirt uses this format since forever, this format never worked, and nobody ever noticed that this was broken. Well, libvirt just entered freeze for 1.0.3. I think the best course of action on libvirt's side is to patch 1.0.3 to flat-out reject any cpumap that cannot be represented in a syntax understood by qemu 1.4; then a future libvirt can re-add support for whatever new syntax qemu 1.5 deems as appropriate. -- Eric Blake eblake 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] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support
On 02/25/2013 02:08 AM, Peter Krempa wrote: On 02/22/13 19:20, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: This patch documents XML elements used for (basic) support of virtual RNG devices. For the slightly more advanced EGD backend: devices rng model='virtio' backend model='egd' type='udp' !-- this is a definition of a character device -- source mode='bind' service='1234'/ source mode='connect' host='1.2.3.4' service='1234'/ !-- or other valid character device configuration -- Do we really want two source in a single backend in the example, or would it be easier to show multiple rng devices, one for each type of backend? That actually is valid for the character device backends. The UDP backend has to use two separate sources for bi-directional communication. The definition of that source type is declared as a type in our RNG schema an I merely reused that. Oh, I didn't realize that. Do we want to ENFORCE that there are two source elements present then? Or can we always provide a sane default when the user provides only 0 or 1 source? Depending on that answer, it might be worth additional documentation on why two source elements are present, as well as mentioning that order is significant in how they are listed. -- Eric Blake eblake 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] [Qemu-devel] [PATCH] vl.c: Support multiple CPU ranges on -numa option
Am 21.02.2013 21:57, schrieb Eduardo Habkost: On Thu, Feb 21, 2013 at 09:23:22PM +0100, Markus Armbruster wrote: Eduardo Habkost ehabk...@redhat.com writes: This allows , to be used a separator between each CPU range. Note that commas inside key=value command-line options have to be escaped using ,,, so the command-line will look like: -numa node,cpus=A,,B,,C,,D This is really, really ugly, and an embarrassment to document. Which you didn't ;) I was trying to have an intermediate solution using the current -numa parser. I have patches in my queue that will change the code to properly use QemuOpts later. Speaking of which, have you considered using QemuOpts for -cpu? Its custom parsing code will probably not handle , escaping at all. ;) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 7/8] tests: Add tests for virtio-rng device handling
On 02/25/2013 03:45 AM, Peter Krempa wrote: On 02/23/13 01:29, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: Adds XML parsing and qemu commandline tests for the VirtIO RNG device support. --- Is it worth testing that a filename containing an XML-special character is properly escaped? Other than that, this one is still good to go. +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml @@ -0,0 +1,23 @@ +rng model='virtio' + backend model='random'/test/phile/backend That is, should this use something like /test/lt;phile as the XML encoded file name? Uh, I'm not following you on this one. You mean that if the user specifies some characters that are invalid from the perspective of XML as a source path? We should not get in the way of a user doing: ln -s /dev/urandom '/tmp/myevilrandom' then passing: backend model='random'/tmp/mylt;evilgt;random/backend in their domain XML. By using virBufferEscape on the output side, we allow the user to use XML escapes on their input to specify any valid file name. Anyways, I fixed the issues you pointed out in 1-6 and provided explanation for the other stuff. I'm pushing patches 1-7 (the test suite can be improved at any time) now and will follow up later with a improved version of 8 as well as with a patch that will allow multiple RNG devices. That should be better to review as slicing apart the existing patches. Sure, doing things as followups is fine, since you already collected ACKs before my late review. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: enable direct migration over IPv6
On 02/25/2013 07:15 AM, Ján Tomko wrote: Use virURIParse in qemuMigrationPrepareDirect to allow parsing IPv6 addresses, which would cause an 'incorrect :port' error message before. To be able to migrate over IPv6, QEMU needs to listen on [::] instead of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen address based on the result. It also uses the same listen address for the NBD server. This will break migration if a hostname does not resolve to the same address family on both sides. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013 --- Diff to V1: * initialize uri_str * reuse STRSKIP(tcp:) result instead of doing strlen on it * print a warning instead of failing when the hostname can't be resolved Diff to V2: * freeaddrinfo * separate the listen address to allow reuse in qemuMigrationStartNBDServer src/qemu/qemu_migration.c | 77 --- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index cae58fa..ff9b959 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -22,7 +22,10 @@ #include config.h +#include netdb.h +#include sys/socket.h #include sys/time.h +#include sys/types.h You rarely ever need sys/types.h - most types in that header are guaranteed to be provided by any other standard header that has an interface using that type; or put another way, POSIX says that inclusion of other headers may drag in sys/types.h, and we have already been relying on that reliably happening. The other two additions are important, though. @@ -2292,9 +2296,14 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, static int port = 0; int this_port; char *hostname = NULL; +char listenAddr[8]; char migrateFrom [64]; As long as you are touching this, remove the space before [64]. Then again, 64 was a magic number; a much safer approach would have been char migrateFrom[sizeof(tcp:0.0.0.0:) + INT_BUFSIZE_BOUND(int)]; (using gnulib's intprops.h), or even getting rid of the stack-alloc temporary (see below for thoughts about using virBufferPtr instead) +if (getaddrinfo(hostname, NULL, NULL, info)) { +VIR_WARN(unable to get address info for %s, defaulting to IPv4, + hostname); Would it be nice to include the gai_strerror() reason for why getaddrinfo() failed? +} else { +ipv6 = info-ai_family == AF_INET6; +freeaddrinfo(info); +} + if (*uri_out) VIR_DEBUG(Generated uri_out=%s, *uri_out); -/* QEMU will be started with -incoming tcp:0.0.0.0:port */ -snprintf(migrateFrom, sizeof(migrateFrom), tcp:0.0.0.0:%d, this_port); +/* QEMU will be started with -incoming tcp:0.0.0.0:port + * or -incoming tcp:[::]:port for IPv6 */ +if (ipv6) +snprintf(listenAddr, sizeof(listenAddr), [::]); +else +snprintf(listenAddr, sizeof(listenAddr), 0.0.0.0); Eww. Even though this is safe (because you sized listenAddr big enough), I'd rather see: const char *listenAddr; if (ipv6) listenAddr = [::]; else listenAddr = 0.0.0.0; + +snprintf(migrateFrom, sizeof(migrateFrom), + tcp:%s:%d, listenAddr, this_port); And while this snprintf was pre-existing and also happens to be large enough, I can't help but think it would be nicer to pass a virBufferPtr instead of a const char * to qemuMigrationPrepareAny, although that would mean more refactoring done in an independent patch. -- Eric Blake eblake 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] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support
On 02/25/13 22:01, Eric Blake wrote: On 02/25/2013 02:08 AM, Peter Krempa wrote: On 02/22/13 19:20, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: This patch documents XML elements used for (basic) support of virtual RNG devices. For the slightly more advanced EGD backend: devices rng model='virtio' backend model='egd' type='udp' !-- this is a definition of a character device -- source mode='bind' service='1234'/ source mode='connect' host='1.2.3.4' service='1234'/ !-- or other valid character device configuration -- Do we really want two source in a single backend in the example, or would it be easier to show multiple rng devices, one for each type of backend? That actually is valid for the character device backends. The UDP backend has to use two separate sources for bi-directional communication. The definition of that source type is declared as a type in our RNG schema an I merely reused that. Oh, I didn't realize that. Do we want to ENFORCE that there are two source elements present then? Or can we always provide a sane default when the user provides only 0 or 1 source? Depending on that answer, it might be worth additional documentation on why two source elements are present, as well as mentioning that order is significant in how they are listed. I reused the code to facilitate the operations done with character devices. There's present infrastructure [1] that allows to parse, format and create commandlines for qemu. The defaults for the syntax of the XML and the expected values should be documented in the section describing character devices. (But I did not really check if the description is exhaustive) In hindsight I should have linked that section in the description of the egd backend model explicitly rather than by just mentioning it. Peter [1]: virDomainChrSourceDefParseXML, virDomainChrSourceDefFormat, qemuBuildChrChardevStr -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Avoid leaking of RNG device definition
--- Yet another place not guarded by the compiler :( This should qualify as trivial, so I'm pushing it directly. src/conf/domain_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3cb780d..b65e52a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1775,6 +1775,8 @@ void virDomainDefFree(virDomainDefPtr def) virDomainRedirdevDefFree(def-redirdevs[i]); VIR_FREE(def-redirdevs); +virDomainRNGDefFree(def-rng); + VIR_FREE(def-os.type); VIR_FREE(def-os.machine); VIR_FREE(def-os.init); -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/8] doc: schema: Add basic documentation for the virtual RNG device support
On 02/25/13 22:20, Peter Krempa wrote: On 02/25/13 22:01, Eric Blake wrote: On 02/25/2013 02:08 AM, Peter Krempa wrote: On 02/22/13 19:20, Eric Blake wrote: On 02/21/2013 07:47 AM, Peter Krempa wrote: This patch documents XML elements used for (basic) support of virtual RNG devices. For the slightly more advanced EGD backend: devices rng model='virtio' backend model='egd' type='udp' !-- this is a definition of a character device -- source mode='bind' service='1234'/ source mode='connect' host='1.2.3.4' service='1234'/ !-- or other valid character device configuration -- Do we really want two source in a single backend in the example, or would it be easier to show multiple rng devices, one for each type of backend? That actually is valid for the character device backends. The UDP backend has to use two separate sources for bi-directional communication. The definition of that source type is declared as a type in our RNG schema an I merely reused that. Oh, I didn't realize that. Do we want to ENFORCE that there are two source elements present then? Or can we always provide a sane default when the user provides only 0 or 1 source? Depending on that answer, it might be worth additional documentation on why two source elements are present, as well as mentioning that order is significant in how they are listed. I reused the code to facilitate the operations done with character devices. There's present infrastructure [1] that allows to parse, format and create commandlines for qemu. The defaults for the syntax of the XML and the expected values should be documented in the section describing character devices. (But I did not really check if the description is exhaustive) In hindsight I should have linked that section in the description of the egd backend model explicitly rather than by just mentioning it. I actually did that: backend type='egd' (Hm there's a mistake here - s/type/model/, I'll fix that) This backend connects to a source using the EGD protocol. The source is specified as a character device. Refer to character device host interface for more information. (where character device host interface links to : http://libvirt.org/formatdomain.html#elementsCharHostInterface ) Peter [1]: virDomainChrSourceDefParseXML, virDomainChrSourceDefFormat, qemuBuildChrChardevStr -- 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
[libvirt] [PATCH 0/4] Virtio-RNG followup
This series adds rate limiting and multiple device support and fixes docs and adds test for XML entities and multiple RNG devices. Peter Krempa (4): virtio-rng: allow multiple RNG devices docs: Fix attribute name for virtio-rng backend virtio-rng: Add rate limiting options for virtio-RNG tests: Test multiple RNG devices and support for XML entities in paths docs/formatdomain.html.in | 14 +++- docs/schemas/domaincommon.rng | 18 - src/conf/domain_conf.c | 78 +- src/conf/domain_conf.h | 6 +- src/qemu/qemu_command.c| 28 +--- .../qemuxml2argv-virtio-rng-random.args| 2 +- .../qemuxml2argv-virtio-rng-random.xml | 4 ++ 7 files changed, 106 insertions(+), 44 deletions(-) -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] virtio-rng: Add rate limiting options for virtio-RNG
Qemu's implementation of virtio RNG supports rate limiting of the entropy used. This patch exposes the option to tune this fucntionality. This patch is based on qemu commit 904d6f588063fb5ad2b61998acdf1e73fb4 The rate limiting is exported in the XML as: devices ... rng model='virtio' rate period='1234'4321/rate backend model='random'/ /rng ... --- Notes: This series would benefit from the per-driver XML parsing checks to verify that rate 8bits, otherwise it will be rounded down to 0 bytes. I will follow up with that change as soon as the per-driver callbacks get in. Version 3: - State the time unit in docs Version 2: - Qemu uses bytes/period, adapt the value according to that docs/formatdomain.html.in | 10 ++ docs/schemas/domaincommon.rng | 18 +- src/conf/domain_conf.c | 17 + src/conf/domain_conf.h | 2 ++ src/qemu/qemu_command.c| 9 + .../qemuxml2argv-virtio-rng-random.args| 2 +- .../qemuxml2argv-virtio-rng-random.xml | 1 + 7 files changed, 57 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 2a60f61..220884c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4294,6 +4294,7 @@ qemu-kvm -net nic,model=? /dev/null ... lt;devicesgt; lt;rng model='virtio'gt; + lt;rate period=2000gt;1234lt;/rategt; lt;backend model='random'gt;/dev/randomlt;/backendgt; lt;!-- OR --gt; lt;backend model='egd' type='udp'gt; @@ -4316,6 +4317,15 @@ qemu-kvm -net nic,model=? /dev/null li'virtio' mdash; supported by qemu and virtio-rng kernel module/li /ul /dd + dtcoderate/code/dt + dd +p + The rate element allows to limit the rate that the entropy can be + read from the source. The value is in bits that the device is allowed + to read in the selected period. The period is represented in miliseconds. + The default period is 1000ms or 1 second. +/p + /dd dtcodebackend/code/dt dd p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8330a50..da53095 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3522,7 +3522,12 @@ valuevirtio/value /choice /attribute - ref name=rng-backend/ + interleave +ref name=rng-backend/ +optional + ref name=rng-rate/ +/optional + /interleave /element /define @@ -3546,6 +3551,17 @@ /element /define + define name=rng-rate +element name=rate + optional +attribute name=period + ref name=positiveInteger/ +/attribute + /optional + ref name=positiveInteger/ +/element + /define + define name=usbmaster element name=master attribute name=startport diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 40eded6..0e2f1a9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7506,6 +7506,17 @@ virDomainRNGDefParseXML(const xmlNodePtr node, ctxt-node = node; +if (virXPathUInt(string(./rate), ctxt, def-rate) -1) { +virReportError(VIR_ERR_XML_ERROR, %s, _(invalid RNG rate value)); +goto error; +} + +if (def-rate 0 +virXPathUInt(string(./rate/@period), ctxt, def-period) -1) { +virReportError(VIR_ERR_XML_ERROR, %s, _(invalid RNG period value)); +goto error; +} + if ((nbackends = virXPathNodeSet(./backend, ctxt, backends)) 0) goto error; @@ -13812,6 +13823,12 @@ virDomainRNGDefFormat(virBufferPtr buf, const char *backend = virDomainRNGBackendTypeToString(def-backend); virBufferAsprintf(buf, rng model='%s'\n, model); +if (def-rate) { +virBufferAddLit(buf, rate); +if (def-period) +virBufferAsprintf(buf, period='%u', def-period); +virBufferAsprintf(buf, %u/rate\n, def-rate); +} virBufferAsprintf(buf, backend model='%s', backend); switch ((enum virDomainRNGBackend) def-backend) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5dc3400..92130f0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1736,6 +1736,8 @@ enum virDomainRNGBackend { struct _virDomainRNGDef { int model; int backend; +unsigned int rate; +unsigned int period; union { char *file; /* file name for 'random' source */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9270258..4899ccf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4256,6 +4256,15 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd, virBufferAsprintf(buf,
[libvirt] [PATCH 1/4] virtio-rng: allow multiple RNG devices
qemu supports adding multiple RNG devices. This patch allows libvirt to support this. --- src/conf/domain_conf.c | 61 + src/conf/domain_conf.h | 4 +++- src/qemu/qemu_command.c | 19 +++ 3 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b65e52a..40eded6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1775,7 +1775,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainRedirdevDefFree(def-redirdevs[i]); VIR_FREE(def-redirdevs); -virDomainRNGDefFree(def-rng); +for (i = 0; i def-nrngs; i++) +virDomainRNGDefFree(def-rngs[i]); +VIR_FREE(def-rngs); VIR_FREE(def-os.type); VIR_FREE(def-os.machine); @@ -2357,10 +2359,10 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, if (cb(def, device, def-memballoon-info, opaque) 0) return -1; } -if (def-rng) { -device.type = VIR_DOMAIN_DEVICE_RNG; -device.data.rng = def-rng; -if (cb(def, device, def-rng-info, opaque) 0) +device.type = VIR_DOMAIN_DEVICE_RNG; +for (i = 0; i def-nrngs; i++) { +device.data.rng = def-rngs[i]; +if (cb(def, device, def-rngs[i]-info, opaque) 0) return -1; } device.type = VIR_DOMAIN_DEVICE_HUB; @@ -10733,21 +10735,21 @@ virDomainDefParseXML(virCapsPtr caps, } } -/* Parse the RNG device */ +/* Parse the RNG devices */ if ((n = virXPathNodeSet(./devices/rng, ctxt, nodes)) 0) goto error; - -if (n 1) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(only a single RNG device is supported)); -goto error; -} - -if (n 0) { -if (!(def-rng = virDomainRNGDefParseXML(nodes[0], ctxt, flags))) +if (n VIR_ALLOC_N(def-rngs, n) 0) +goto no_memory; +for (i = 0; i n; i++) { +virDomainRNGDefPtr rng = virDomainRNGDefParseXML(nodes[i], + ctxt, + flags); +if (!rng) goto error; -VIR_FREE(nodes); + +def-rngs[def-nrngs++] = rng; } +VIR_FREE(nodes); /* analysis of the hub devices */ if ((n = virXPathNodeSet(./devices/hub, ctxt, nodes)) 0) { @@ -11702,17 +11704,6 @@ static bool virDomainRNGDefCheckABIStability(virDomainRNGDefPtr src, virDomainRNGDefPtr dst) { -if (!src !dst) -return true; - -if (!src || !dst) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(Target domain RNG device count '%d' - does not match source count '%d'), - src ? 1 : 0, dst ? 1 : 0); -return false; -} - if (src-model != dst-model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(Target RNG model '%s' does not match source '%s'), @@ -12129,8 +12120,16 @@ virDomainDefCheckABIStability(virDomainDefPtr src, dst-memballoon)) return false; -if (!virDomainRNGDefCheckABIStability(src-rng, dst-rng)) +if (src-nrngs != dst-nrngs) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Target domain RNG device count %zu + does not match source %zu), dst-nrngs, src-nrngs); return false; +} + +for (i = 0 ; i src-nrngs ; i++) +if (!virDomainRNGDefCheckABIStability(src-rngs[i], dst-rngs[i])) +return false; return true; } @@ -15054,8 +15053,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def-memballoon) virDomainMemballoonDefFormat(buf, def-memballoon, flags); -if (def-rng) -virDomainRNGDefFormat(buf, def-rng, flags); +for (n = 0; n def-nrngs; n++) { +if (virDomainRNGDefFormat(buf, def-rngs[n], flags)) +goto error; +} virBufferAddLit(buf, /devices\n); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0828954..5dc3400 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1878,13 +1878,15 @@ struct _virDomainDef { size_t nseclabels; virSecurityLabelDefPtr *seclabels; +size_t nrngs; +virDomainRNGDefPtr *rngs; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; -virDomainRNGDefPtr rng; void *namespaceData; virDomainXMLNamespace ns; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1c9bfc9..9270258 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -787,8 +787,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if
[libvirt] [PATCH 4/4] tests: Test multiple RNG devices and support for XML entities in paths
Users may want to specify XML entities in paths to devices. Ensure they are parsed and used properly. Also test support for multiple RNG devices in one guest. --- tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args index ced11db..a99931a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -object rng-random,id=rng0,filename=/test/phile -device virtio-rng-pci,rng=rng0,max-bytes=100,period=1234,bus=pci.0,addr=0x4 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -object rng-random,id=rng0,filename=/test/phile -device virtio-rng-pci,rng=rng0,max-bytes=100,period=1234,bus=pci.0,addr=0x4 -object 'rng-random,id=rng1,filename=/test/strangephile' -device virtio-rng-pci,rng=rng1,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml index 26ddd38..9950f5b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml @@ -20,5 +20,8 @@ rate period='1234'800/rate backend model='random'/test/phile/backend /rng +rng model='virtio' + backend model='random'/test/strangelt;gt;phile/backend +/rng /devices /domain -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/16] conf: Refactor ABI stability checking and break long lines
On 02/21/13 15:27, Peter Krempa wrote: On 02/20/13 18:44, Laine Stump wrote: On 02/20/2013 12:06 PM, Peter Krempa wrote: Get rid of the identical variable in the ABI stability checks in favor of return statements and break or refactor very long lines where possible. --- src/conf/domain_conf.c | 469 + 1 file changed, 200 insertions(+), 269 deletions(-) ACK (although in general I have some misgivings about cosmetic changes like this, as they obscure the history of the modified lines, making it more tedious to sift through the past commit logs to understand why certain bits of code exist / what changed, etc). Thankfully there's a script for vim (called fugitive) and for sure for other editors too that allows to browse the blame history. Thanks for the reviews, I pushed patches 1-6 and 8. Peter Ping? Could anyone please have a look at the rest of the series. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] docs: Fix attribute name for virtio-rng backend
--- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 12c9468..2a60f61 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4328,7 +4328,7 @@ qemu-kvm -net nic,model=? /dev/null li'egd' mdash; a EGD protocol backend/li /ul /dd - dtcodebackend type='random'/code/dt + dtcodebackend model='random'/code/dt dd p This backend type expects a non-blocking character device as input. @@ -4337,7 +4337,7 @@ qemu-kvm -net nic,model=? /dev/null When no file name is specified the hypervisor default is used. /p /dd - dtcodebackend type='egd'/code/dt + dtcodebackend model='egd'/code/dt dd p This backend connects to a source using the EGD protocol. -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RESEND] ESX: Fix DISPATCH_FREE generation code to free all extended objects
ping ... From: ata.hus...@hotmail.com To: libvir-list@redhat.com CC: ata.hus...@hotmail.com Subject: [PATCH] ESX: Fix DISPATCH_FREE generation code to free all extended objects Date: Tue, 1 Jan 2013 22:22:28 -0800 Python code generator generate_source section that handles code generation to free inherited objects needs to generate DISPATCH_FREE calls for all extended_by objects. --- src/esx/esx_vi_generator.py | 21 ++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index af4e7e8..7a7cf76 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -4,6 +4,7 @@ # esx_vi_generator.py: generates most of the SOAP type mapping code # # Copyright (C) 2010-2012 Matthias Bolte matthias.bo...@googlemail.com +# Copyright (C) 2013 Ata E Husain Bohra ata.hus...@hotmail.com # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -785,9 +786,7 @@ class Object(Type): source += ESX_VI__TEMPLATE__DYNAMIC_FREE(%s,\n % self.name source += {\n -for extended_by in self.extended_by: -source += ESX_VI__TEMPLATE__DISPATCH__FREE(%s)\n \ - % extended_by +source += recurse_dispatch(self, ESX_VI__TEMPLATE__DISPATCH__FREE) source += },\n source += {\n @@ -1285,6 +1284,22 @@ class ManagedObject(Type): +def recurse_dispatch(object, text): + +source = + +if object.extended_by is None: +return source + +for extended_by in object.extended_by: +source += %s(%s)\n % (text, extended_by) +child = objects_by_name[extended_by] +source += recurse_dispatch(child, text) + +return source + + + class Enum(Type): FEATURE__ANY_TYPE = (1 1) FEATURE__SERIALIZE = (1 2) -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RESEND] ESX: Add AnyType_Serialize routine to esx_vi_types.c
ping .. From: ata.hus...@hotmail.com To: matthias.bo...@googlemail.com Date: Tue, 1 Jan 2013 12:07:12 -0800 CC: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH] ESX: Add AnyType_Serialize routine to esx_vi_types.c --- src/esx/esx_vi_types.c | 48 src/esx/esx_vi_types.h |3 ++- 2 files changed, 50 insertions(+), 1 deletion(-) Looks good on a quick view, but I wonder what are you trying to that requires to serialize an AnyType directly instead of the specific types? Some of the variables contains anytype variables that are needed during serialization time such as: extraConfig field of VirtualMachineConfigSpec or ArrayUpdateSpec. I found that without providing serialize method even compilation does not work. -- Matthias Bolte http://photron.blogspot.com -- 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
[libvirt] Hard requirement on gcrypt.h?
Hi, I'm building libvirt for the first time and figuring out what libraries are required to get the simplest possible thing working. Is there a hard requirement on gcrypt? I got libvirt past the configure script but the build failed on gcrypt.h. It looks like the include for it could be moved inside the #if WITH_GNUTLS section of libirt.c, if I understand what configure.ac is trying to set up. Thoughts? Thanks! Eugene -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] tests: uniformly report test failures
On 02/25/2013 03:38 AM, Michal Privoznik wrote: On 23.02.2013 00:09, Eric Blake wrote: testutils.c likes to print summaries after a test completes, including if it failed. But if the test outright exit()s, this summary is skipped. Enforce that we return instead of exit. ACK Thanks for the reviews. Series pushed now, as this improves the testsuite for 1.0.3. -- Eric Blake eblake 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] Hard requirement on gcrypt.h?
On 02/25/2013 05:38 PM, Eugene Marcotte wrote: Hi, I'm building libvirt for the first time and figuring out what libraries are required to get the simplest possible thing working. Is there a hard requirement on gcrypt? I got libvirt past the configure script but the build failed on gcrypt.h. It looks like the include for it could be moved inside the #if WITH_GNUTLS section of libirt.c, if I understand what configure.ac is trying to set up. Can you show the exact compiler error message you received? You are correct that this header is non-standard, and should be properly ifdef'd before we try to use it. -- Eric Blake eblake 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] wrap some long lines
ping ... 在 2013-02-19二的 10:22 +0800,liguang写道: Signed-off-by: liguang lig.f...@cn.fujitsu.com --- src/qemu/qemu_command.c | 71 ++ 1 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9ad3c77..ad62120 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -611,7 +611,8 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def, virDomainNetDefPtr net, int idx) } if ((thisidx = qemuDomainDeviceAliasIndex(def-nets[i]-info, net)) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Unable to determine device index for network device)); + _(Unable to determine device index for + network device)); return -1; } if (thisidx = idx) @@ -638,7 +639,8 @@ qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hostdev int thisidx; if ((thisidx = qemuDomainDeviceAliasIndex(def-hostdevs[i]-info, hostdev)) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Unable to determine device index for hostdev device)); + _(Unable to determine device index for + hostdev device)); return -1; } if (thisidx = idx) @@ -665,7 +667,8 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redir int thisidx; if ((thisidx = qemuDomainDeviceAliasIndex(def-redirdevs[i]-info, redir)) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Unable to determine device index for redirected device)); + _(Unable to determine device index for + redirected device)); return -1; } if (thisidx = idx) @@ -1001,8 +1004,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, if (info-addr.pci.function != 0) { virReportError(VIR_ERR_XML_ERROR, _(Attempted double use of PCI Address '%s' - (may need \multifunction='on'\ for device on function 0)), - addr); + (may need \multifunction='on'\ for + device on function 0)), addr); } else { virReportError(VIR_ERR_XML_ERROR, _(Attempted double use of PCI Address '%s'), addr); @@ -1036,7 +1039,8 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, goto cleanup; } -VIR_DEBUG(Remembering PCI addr %s (multifunction=off for function 0), addr); +VIR_DEBUG(Remembering PCI addr %s (multifunction=off + for function 0), addr); if (virHashAddEntry(addrs-used, addr, addr)) goto cleanup; addr = NULL; @@ -1468,7 +1472,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, def-controllers[i]-info.addr.pci.slot != 1 || def-controllers[i]-info.addr.pci.function != 2) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(PIIX3 USB controller must have PCI address 0:0:1.2)); + _(PIIX3 USB controller must have + PCI address 0:0:1.2)); goto error; } reservedUSB = true; @@ -1524,7 +1529,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, primaryVideo-info.addr.pci.slot != 2 || primaryVideo-info.addr.pci.function != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Primary video card must have PCI address 0:0:2.0)); + _(Primary video card must have + PCI address 0:0:2.0)); goto error; } /* If TYPE==PCI, then qemuCollectPCIAddress() function @@ -1814,12 +1820,14 @@ qemuBuildRomStr(virBufferPtr buf, if (info-rombar || info-romfile) { if (info-type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - %s, _(rombar and romfile are supported only for PCI devices)); + %s, _(rombar and romfile are supported + only for PCI devices)); return -1; } if (!qemuCapsGet(caps, QEMU_CAPS_PCI_ROMBAR)) {
Re: [libvirt] Hard requirement on gcrypt.h?
On Mon, 2013-02-25 at 17:42 -0700, Eric Blake wrote: On 02/25/2013 05:38 PM, Eugene Marcotte wrote: Hi, I'm building libvirt for the first time and figuring out what libraries are required to get the simplest possible thing working. Is there a hard requirement on gcrypt? I got libvirt past the configure script but the build failed on gcrypt.h. It looks like the include for it could be moved inside the #if WITH_GNUTLS section of libirt.c, if I understand what configure.ac is trying to set up. Can you show the exact compiler error message you received? You are correct that this header is non-standard, and should be properly ifdef'd before we try to use it. make[3]: Entering directory `/home/emarcotte/src/libvirt/src' CC libvirt_driver_la-libvirt.lo libvirt.c:34:20: fatal error: gcrypt.h: No such file or directory compilation terminated. Of course, installing gcrypt-devel fixes it, but I would have expected configure to fail. I can try to add an explicit check for gcrypt.h if it's required. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] Add NVRAM device
Daniel/Eric, Any suggestion about this patch? On 2013年02月25日 13:49, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device. Users are allowed to specify spapr-vio devices'address. But NVRAM is not supported in libvirt. So this patch is to add NVRAM device to allow users to specify its address. In QEMU, NVRAM device's address is specified by -global spapr-nvram.reg=x. In libvirt, XML file is defined as the following: nvram address type='spapr-vio' reg='0x3000'/ /nvram Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- * v2 - v1: Add NVRAM parameters parsing in qemuParseCommandLine. src/conf/domain_conf.c | 83 ++- src/conf/domain_conf.h | 10 ++ src/qemu/qemu_command.c | 48 +++ src/qemu/qemu_command.h |2 ++ 4 files changed, 142 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10f361c..8c1e8ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -175,7 +175,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, redirdev, smartcard, chr, - memballoon) + memballoon, + nvram) VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, none, @@ -1442,6 +1443,16 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) VIR_FREE(def); } +void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) +{ +if (!def) +return; + +virDomainDeviceInfoClear(def-info); + +VIR_FREE(def); +} + void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def) { if (!def) @@ -1602,6 +1613,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_MEMBALLOON: +case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_LAST: break; } @@ -1791,6 +1803,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def-watchdog); virDomainMemballoonDefFree(def-memballoon); +virDomainNVRAMDefFree(def-nvram); for (i = 0; i def-nseclabels; i++) virSecurityLabelDefFree(def-seclabels[i]); @@ -2342,6 +2355,12 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, if (cb(def, device, def-memballoon-info, opaque) 0) return -1; } +if (def-nvram) { +device.type = VIR_DOMAIN_DEVICE_NVRAM; +device.data.nvram = def-nvram; +if (cb(def, device, def-nvram-info, opaque) 0) +return -1; +} device.type = VIR_DOMAIN_DEVICE_HUB; for (i = 0; i def-nhubs ; i++) { device.data.hub = def-hubs[i]; @@ -7461,6 +7480,23 @@ error: goto cleanup; } +static virDomainNVRAMDefPtr +virDomainNVRAMDefParseXML(const xmlNodePtr node, + unsigned int flags) +{ + virDomainNVRAMDefPtr def; + +if (VIR_ALLOC(def) 0) { +virReportOOMError(); +return NULL; +} + +if ( virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0 ) +return NULL; + +return def; +} + static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt) @@ -10572,6 +10608,32 @@ virDomainDefParseXML(virCapsPtr caps, } } +def-nvram = NULL; +if ((n = virXPathNodeSet(./devices/nvram, ctxt, nodes)) 0) { +goto error; +} + +if (n 1) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(only a single nvram device is supported)); +goto error; +} + +if (n 0) { +virDomainNVRAMDefPtr nvram = +virDomainNVRAMDefParseXML(nodes[0], flags); +if (!nvram) +goto error; +def-nvram = nvram; +VIR_FREE(nodes); +} else { +virDomainNVRAMDefPtr nvram; +if (VIR_ALLOC(nvram) 0) +goto no_memory; +nvram-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; +def-nvram = nvram; +} + /* analysis of the hub devices */ if ((n = virXPathNodeSet(./devices/hub, ctxt, nodes)) 0) { goto error; @@ -13547,6 +13609,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf, } static int +virDomainNVRAMDefFormat(virBufferPtr buf, + virDomainNVRAMDefPtr def, + unsigned int flags) +{ +virBufferAsprintf(buf, nvram\n); +if (virDomainDeviceInfoIsSet(def-info, flags)) +if (virDomainDeviceInfoFormat(buf, def-info, flags) 0) +return -1; + + virBufferAddLit(buf, /nvram\n); + + return 0; +} + +static int virDomainSysinfoDefFormat(virBufferPtr buf, virSysinfoDefPtr def) { @@ -14787,6 +14864,9 @@
Re: [libvirt] [PATCHv2 1/1] Optimize machine option to set more options with it.
Hi Daniel, This patch is still use USB option to disable the controller which created in machine init of QEMU. Because with USB model none, other USB devices can't be used to attach USB controller. So we disable the controller in machine init and choose the controller defined in libvirt by -device . That's why I send it again with some fixes. Can you consider about it? Thanks a lot. -Li On 2013年02月22日 17:09, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com Currently, -machine option is used only when dump-guest-core is used. To use options defined in machine option for new version of QEMU, it needs to use -machine xxx, and to be compatible with older version -M, this patch addes QEMU_CAPS_MACH_OPT capability, and assumes -machine is used for QEMU v1.0 onwards. To avoid the collision for creating USB controllers when using USB option and -device , it needs to set usb=off in machine option. QEMU_CAPS_USB_OPT capability is added, and it will be for QEMU v1.3.0-rc0 onwards which supports USB option. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- src/qemu/qemu_capabilities.c | 10 ++ src/qemu/qemu_capabilities.h |2 ++ src/qemu/qemu_command.c | 36 +--- tests/qemuxml2argvtest.c |4 ++-- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 51fc9dc..79eb83f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -205,6 +205,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, usb-serial, /* 125 */ usb-net, add-fd, + mach-opt, + usb-opt, ); @@ -2416,6 +2418,14 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, virQEMUCapsInitQMPBasic(qemuCaps); +/* Assuming to use machine option v1.0 onwards*/ +if (qemuCaps-version = 100) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_OPT); + +/* USB option is supported v1.3.0-rc0 onwards */ +if (qemuCaps-version = 1002090) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_USB_OPT); + if (!(archstr = qemuMonitorGetTargetArch(mon))) goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e69d558..06aaa68 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -166,6 +166,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_USB_SERIAL = 125, /* -device usb-serial */ QEMU_CAPS_DEVICE_USB_NET = 126, /* -device usb-net */ QEMU_CAPS_ADD_FD = 127, /* -add-fd */ +QEMU_CAPS_MACH_OPT = 128, /* -machine */ +QEMU_CAPS_USB_OPT= 129, /* -machine ,usb=off*/ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c28123..e7dde21 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4614,6 +4614,8 @@ qemuBuildMachineArgStr(virCommandPtr cmd, const virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { +virBuffer buf = VIR_BUFFER_INITIALIZER; + /* This should *never* be NULL, since we always provide * a machine in the capabilities data for QEMU. So this * check is just here as a safety in case the unexpected @@ -4621,27 +4623,39 @@ qemuBuildMachineArgStr(virCommandPtr cmd, if (!def-os.machine) return 0; -if (!def-mem.dump_core) { +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACH_OPT)) { /* if no parameter to the machine type is needed, we still use * '-M' to keep the most of the compatibility with older versions. */ virCommandAddArgList(cmd, -M, def-os.machine, NULL); } else { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - %s, _(dump-guest-core is not available -with this QEMU binary)); -return -1; -} /* However, in case there is a parameter to be added, we need to * use the -machine parameter because qemu is not parsing the * -M correctly */ + virCommandAddArg(cmd, -machine); -virCommandAddArgFormat(cmd, - %s,dump-guest-core=%s, - def-os.machine, - virDomainMemDumpTypeToString(def-mem.dump_core)); +virBufferAsprintf(buf, %s, def-os.machine); + +/* To avoid the collision of creating USB controllers when calling + * machine-init in QEMU, it needs to set usb=off + */ +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_OPT)) +virBufferAsprintf(buf, ,usb=off); + +if (def-mem.dump_core) { +if (!virQEMUCapsGet(qemuCaps,
Re: [libvirt] [Dnsmasq-discuss] Chaining instances?
2013/2/25 Laine Stump la...@laine.org On 02/25/2013 02:05 AM, Laine Stump wrote: This discussion should really be taking place on libvir-list - I'm Cc'ing it there. Bah. I'm not sure what happened, but I ended up not actually Cc'ing... On 02/24/2013 04:11 PM, TJ wrote: On 24/02/13 19:19, Laine Stump wrote: On 02/24/2013 05:09 AM, TJ wrote: I wondered if maybe configuring the libvirtd dnsmasq instances to be dhcp proxies for the LAN dnsmasq, and use multiple dhcp-range's with tags might do it? My brain is a bit fried right now having had to fix several bugs in vmbuilder and libvirt, plus add new functionality to libvirtd to allow its dnsmasq instance to read a conf-file and use a separate log-facility, What you're talking about doing sounds *very* useful to have supported directly in libvirt, but you may want to contact libvirt's upstream developers (at libvir-list@redhat.com, or on irc.oftc.net in #virt) prior to expending a lot of effort on libvirt code - in general a problem may already be solved in a different manner, or somebody else may already be working on it. Failing that, there may have already been considerable debate on a particular subject, and a path to a solution generally agreed on, but with nobody yet working on the code. It turns out that what I need(ed) to do was completely *disable* libvirt's use of dnsmasq and instead use Simon's related dhcp-helper program which acts as a full dhcp-relay using, f.e: /usr/sbin/dhcp-helper -u libvirt-dnsmasq -i virbr1 -b eth0 This forwards all requests to the primary dnsmasq DHCP server on the LAN which matches against the appropriate dhcp-range: # physical network leases dhcp-range=set:phys,10.254.251.50,10.254.251.199,255.255.255.0,1440m # subnet for VMs on server1 dhcp-range=set:vmlan1,10.254.1.100,10.254.1.199,255.255.255.0,1440m # default route (gateway) dhcp-option=tag:phys,option:router,10.254.251.1 dhcp-option=tag:vmlan1,option:router,10.254.1.1 # DNS server dhcp-option=6,10.254.251.1 To that end this evening I added two additional options to libvirt: dnsmasq enabled='true'/ dhcphelper enabled='false'/ libvirt's XML should remain implementation-agnostic. We don't want to have an option to disable the specific implementation of dns+dhcp called dnsmasq; rather we want to be able to enable/disable a network's dhcp server and/or dns server. (The reason for this is that we want to leave the door open for someone to implement a different backend using a different dhcp server and/or dns server and be able to use the same configuration.) We have actually previously discussed disabling dns and/or dhcp (see http://www.redhat.com/archives/libvir-list/2012-November/msg00861.html). It is already possible to completely disable dhcp - simply don't include a dhcp element in the network definition. As for dns, from the very beginning dns has been *always* enabled on all networks, so even though there is a dns element, simply having not having one is not a valid way to say no dns server (as it would cause backward compatibility problems with existing installations). Instead, the conclusion of the above-mentioned thread was that the proper way to handle this would be to add an enable element to the existing dns element, which would default to yes. To disable libvirt-supplied dns services for a subnet, you would just put: dns enable='no'/ in the network definition. In the case that dns had enable='no' AND there was no dhcp element, dnsmasq simply wouldn't be started at all. That hasn't been implemented yet, but shouldn't be too complex to do. As for dhcp-helper, aside from the fact that again you've created an option that is specific to a particular implementation of what is needed, that command isn't universally available anyway (it's not in any package for Fedora/RHEL, for example), and I'm not sure that it's really necessary to have it started up by libvirt anyway - can it detect newly created/upped interfaces as dnsmasq can? If so, it could be started up by regular host system config even before libvirt was started. These are the default values when the options are *not* defined. They allow the admin to disable dnsmasq entirely: dnsmasq enabled='false'/ and to enable dhcp-helper: dhcphelper enabled='true'/ Using two new functions: int networkBuildDhcphelperArgv(...) int networkBuildDhcpHelperCommandLine(...) the existing function: int networkStartDhcpDaemon(...) is able to launch either or both of dnsmasq and dhcp-helper with the correct options. dhcp-helper's -i option is filled from network-def-bridge and its -b option is taken from the first forward device declared in a forward dev='?'/ or interface dev='?'/. If no forward device is declared it throws a VIR_ERR_INVALID_INTERFACE error with appropriate explanatory text. -- libvir-list
Re: [libvirt] Entering freeze for libvirt-1.0.3
On 2013年02月25日 19:52, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are following too at the usual place: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests, but of course more testing and checking on other architectures is needed ! If everything goes fine, I will make an rc2 in a couple of days and then we can decide if a final release on Friday is fine or if we need to postpone to next week, There are too few changes in this release for pp64. Testing it by building source for ppc64, it works well. Thanks. --Li Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.0.3
On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote: On 2013年02月25日 19:52, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are following too at the usual place: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests, but of course more testing and checking on other architectures is needed ! If everything goes fine, I will make an rc2 in a couple of days and then we can decide if a final release on Friday is fine or if we need to postpone to next week, There are too few changes in this release for pp64. By too few do you mean that we are missing anything important ? Testing it by building source for ppc64, it works well. Cool, thanks for testing :-) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ 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] Entering freeze for libvirt-1.0.3
On 2013年02月26日 12:51, Daniel Veillard wrote: On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote: On 2013年02月25日 19:52, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are following too at the usual place: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests, but of course more testing and checking on other architectures is needed ! If everything goes fine, I will make an rc2 in a couple of days and then we can decide if a final release on Friday is fine or if we need to postpone to next week, There are too few changes in this release for pp64. By too few do you mean that we are missing anything important ? I have sent out 2 patches which are not accepted for this release. One patch is to fix one bug for ppc64 which is: [PATCHv2 1/1] Optimize machine option to set more options with it. Without this patch, USB controllers doesn't work well with VGA enabled for ppc64. And testing with VDSM, it will always report errors when using VGA. Testing it by building source for ppc64, it works well. Cool, thanks for testing :-) Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] The problem of CPU index
Hi all, In the function qemuMonitorJSONExtractCPUInfo(), it assumes that CPU indexes should be contiguous. As the following: if (cpu != i) { virReportError(VIR_ERR_INTERNAL_ERROR, _(unexpected cpu index %d expecting %d), i, cpu); goto cleanup; } For x86, in what kind of situation will cause this error? Actually, on ppc64, the CPU indexes always are not contiguous, because smt is always disabled because of hardware's limitation for KVM. For example, smt=4, CPUs=64, cores=16. If with SMT enabled, 0~3 will run on core 0, 4~7 will run on core 1, If SMT is disabled, only one thread is active. It should be 0, 4, 8, CPU information on my machine is as the following: Architecture: ppc64 Byte Order:Big Endian CPU(s):64 On-line CPU(s) list: 0,4,8,12,16,20,24,28,32,36,40,44,48,52,56,60 Off-line CPU(s) list: 1-3,5-7,9-11,13-15,17-19,21-23,25-27,29-31,33-35,37-39,41-43,45-47,49-51,53-55,57-59,61-63 Thread(s) per core:1 Core(s) per socket:1 Socket(s): 16 NUMA node(s): 2 Model: 8246-L2C L1d cache: 32K L1i cache: 32K L2 cache: 256K L3 cache: 4096K L4 cache: 4096K NUMA node0 CPU(s): 0,36,40,44,48,52,56,60 NUMA node1 CPU(s): 4,8,12,16,20,24,28,32 Actually, only 16 CPUs are online. All we get CPUs are 0,4,8,...,60. If possible, could this assumption be removed? Thanks --Li -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.0.3
On Tue, Feb 26, 2013 at 01:03:17PM +0800, Li Zhang wrote: On 2013年02月26日 12:51, Daniel Veillard wrote: On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote: On 2013年02月25日 19:52, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are following too at the usual place: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests, but of course more testing and checking on other architectures is needed ! If everything goes fine, I will make an rc2 in a couple of days and then we can decide if a final release on Friday is fine or if we need to postpone to next week, There are too few changes in this release for pp64. By too few do you mean that we are missing anything important ? I have sent out 2 patches which are not accepted for this release. One patch is to fix one bug for ppc64 which is: [PATCHv2 1/1] Optimize machine option to set more options with it. Without this patch, USB controllers doesn't work well with VGA enabled for ppc64. And testing with VDSM, it will always report errors when using VGA. Hum, i just read your patch but i don't have enough knowledge of qemu parameters to know if there is large side effect to your patch. Also you remove code which is about core and replace it with something completely different. As is and with the amount of details you provided I can't make any jugement of how bad or good those changes are. One thing is sure, bug fixes can go in, they are fine during freeze ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] interface: udev backend coverity NULL deref
This fixes a potential NULL deref identified by John Ferlan jfer...@redhat.com if scandir() didn't return an expected value. --- src/interface/interface_backend_udev.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index dca85b3..1034429 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -779,6 +779,13 @@ udevIfaceGetIfaceDefBond(struct udev *udev, * so we use the part after the _ */ tmp_str = strchr(slave_list[i]-d_name, '_'); +if (!tmp_str || strlen(tmp_str) 2) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Invalid enslaved interface name '%s' seen for + bond '%s', slave_list[i]-d_name, name)); +goto cleanup; +} +/* go past the _ */ tmp_str++; ifacedef-data.bond.itf[i] = -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.0.3
On 2013年02月26日 14:04, Daniel Veillard wrote: On Tue, Feb 26, 2013 at 01:03:17PM +0800, Li Zhang wrote: On 2013年02月26日 12:51, Daniel Veillard wrote: On Tue, Feb 26, 2013 at 11:10:35AM +0800, Li Zhang wrote: On 2013年02月25日 19:52, Daniel Veillard wrote: I have just tagged git and pushed the tarball. The rpms for F17 are following too at the usual place: ftp://libvirt.org/libvirt/ I gave a try to the set of rpms and this seems to work fine for relatively simple tests, but of course more testing and checking on other architectures is needed ! If everything goes fine, I will make an rc2 in a couple of days and then we can decide if a final release on Friday is fine or if we need to postpone to next week, There are too few changes in this release for pp64. By too few do you mean that we are missing anything important ? I have sent out 2 patches which are not accepted for this release. One patch is to fix one bug for ppc64 which is: [PATCHv2 1/1] Optimize machine option to set more options with it. Without this patch, USB controllers doesn't work well with VGA enabled for ppc64. And testing with VDSM, it will always report errors when using VGA. Hum, i just read your patch but i don't have enough knowledge of qemu parameters to know if there is large side effect to your patch. Also you remove code which is about core and replace it with something completely different. Thanks so much for your reviewing. In qemu, it provides some options for some kind of devices or hardware by -machine option. I think its purpose is to provide the configurations for machines to compare with -device xxx. There will be more and more options to be set by -machine according to QEMU's development. So, in my patch, -machine is used to be set more options not only dump_core with newer QEMU versions As is and with the amount of details you provided I can't make any jugement of how bad or good those changes are. One thing is sure, bug fixes can go in, they are fine during freeze ! Got it, thanks. :) I will wait for more comments from mailing list. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't fail to shutdown domains with unresponsive agent
On 25.02.2013 19:33, Eric Blake wrote: On 02/25/2013 10:55 AM, Michal Privoznik wrote: Currently, qemuDomainShutdownFlags() chooses the agent method of shutdown whenever the agent is configured. However, this assumption is not enough as the guest agent may be unresponsive at the moment. So unless guest agent method has been explicitly requested, we should fall back to the ACPI method. --- src/qemu/qemu_driver.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) When Daniel added two new shutdown methods for LXC, I had the question on whether shutdown methods should be mutually exclusive, or a bitmask of all permissible things to attempt (where passing 0 leaves the attempts up the hypervisor). The consensus was that allowing the user to pass more than one method makes sense (although the hypervisor then gets to choose method priorities). I'm not sure your logic matches the goal of allowing the user to request both agent and acpi at the same time. Aah. Okay; I thought they were considered as mutually exclusive. Hence my code is not correct. Let me re-spin v2. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list