Re: [libvirt] Coverity scan
On 02.01.2013 20:22, John Ferlan wrote: On 12/20/2012 04:24 PM, Eric Blake wrote: On 12/20/2012 08:25 AM, John Ferlan wrote: First allow me to introduce myself - I'm John Ferlan a new Red Hat employee (3 weeks). I came from the closed world at HP where for the last 7 years I worked in a group developing/supporting HP's Integrity Virtual Machine software prior to it being outsourced to India this past May. I primarily worked in the CLI/API and daemon space, although I also spent quite a bit of time in the lower virtualization layers which mimicked the Integrity instructions. I am very happy to be in the open world and look forward to contributing. Everyone has to start some where. Welcome to libvirt! Based on recent list traffic, there are several people, not just John, and not just at Red Hat, that are aiming to provide initial contributions. Be sure to provide your feedback and questions on the list, and hopefully we can use that to make our hacking documentation even easier to follow for future newcomers. You may want to figure out why your mailer doesn't automatically wrap long lines. Webmail interfaces (gmail, zimbra) and Microsoft Exchange tend to be the worst mail agents when it comes to RFC compliance and lack of knobs to tune for improving the situation, which in turn can make it harder to read your messages and apply patches you submit. You may notice that many people on this list use mutt or Thunderbird rather than web mailers, if only to have better formatted mail, but it's not a hard pre-requisite (we won't reject a patch just because of how it was sent, although it might take longer to apply if we have to jump through hoops to get it extracted from the mail). For mailers that have the right knobs, turning on format=flowed is one way of sending reasonably formatted mail [but be aware that Thunderbird has had a rather nasty bug for several years now where defaulting to format=flowed can result in botching the whitespace of the quoted portion of a message you are replying to]. My first task here at Red Hat was to triage a Coverity scan executed against libvirt-1.0.0-1.fc19.src.rpm done in late November. There were 285 issues documented. I quickly found that some of the defects found there were already fixed in later submits upstream, so I ran a new Coverity scan last Friday and it came back with 297 issues broken down as follows: 1 ARRAY_VS_SINGLETON 33 BAD_SIZEOF 17 CHECKED_RETURN 1 CONSTANT_EXPRESSION_RESULT 5 COPY_PASTE_ERROR 13 DEADCODE 46 FORWARD_NULL 2 MISSING_RETURN 2 NEGATIVE_RETURNS 7 NULL_RETURNS 1 OVERRUN 137 RESOURCE_LEAK 18 REVERSE_INULL 1 SIGN_EXTENSION 3 UNINIT 8 UNUSED_VALUE 2 USE_AFTER_FREE Thanks for doing this. Helping to reduce the false positives and eliminate the real bugs will make it easier to run Coverity on a periodic basis and check for recent regressions while the code is still fresh on our minds. Looking forward to the patches you come up with. Ran a new scan recently - the number of defects increased slightly (+4 FORWARD_NULL, +2 MISSING_RETURN, and + 1 UNINIT). I worked my way through the entire list and just figured I'd start with an easier group for my first submit attempt. I have a series of relatively easy changes for the CHECKED_RETURN condition; however, I was hoping to perhaps get some feedback on two files which had more ramifications from just checking the return status. The first is 'src/phyp/phyp_driver.c'. Neither waitsocket() nor any of the callers check the return status. The implication being if select() fails the code will continue to wait. So is this desired? Conversely what are the ramifications of checking status on select() and using virReportSystemError()? Or perhaps some other way to log the failure? For any of the callers, any concerns over checking status return == -1 and jumping to the err label? I guess I'm concerned over making logic/flow changes to some long standing assumption. I've been on the opposite end of debugging one of those. I think we should check for the return value of select() / waitsocket(). But I also think we can silently ignore EINTR. So the code should look something like this: errno = 0; while (some_libssh2-ish_rubbish) { if (waitsocket(...) 0 errno != EINTR) { virReportSystemError(errno, %s, _(unable to wait on libssh2 socket); goto err; } } The second is 'src/rpc/virnetsocket.c'. The Coverity complaint is that the setsockopt() call to set SO_REUSEADDR doesn't check the return status. I think this particular case may be a cut-n-paste type change as the from virNetSocketNewListenTCP(). Unless I'm missing something, does setting the reuse addr on a connect socket do anything? Can it just safely be removed? In fact on Linux (yet another difference from *BSD) it does make sense to set SO_REUSEADDR
[libvirt] Greetings and next release
Hello everybody, Happy new year and best wishes for 2013 ! When we planned the 1.0.1 release we took the option of taking a bit of extra time to push mid-december and planning the following one for the end of January. Based on this I suggest to enter the freeze for libvirt-1.0.2 on the Thursday 24 so that we have a week to push for a final release on the end of the month. We are already over 110 commits since 1.0.1 so 3 weeks from now I would expect enough changes for a interesting release :-) 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] [PATCH] qemu: Don't parse log output when starting up a domain
On 02.01.2013 18:45, Eric Blake wrote: On 01/02/2013 08:23 AM, Michal Privoznik wrote: Despite our great effort we still parsed qemu log output. We wouldn't notice unless qemu changed the format of the logs slightly. Maybe mention that the upcoming qemu 1.4 does just that. Anyway, now we should gather all interesting knobs like pty paths from monitor. Moreover, since for historical reasons the first console can be just an alias to the first serial port, we need to check this and copy the pty path if that's the case to the first console. --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_process.c | 20 ++-- 3 files changed, 26 insertions(+), 2 deletions(-) ACK, with a grammar nit. @@ -1544,8 +1545,23 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, if (qemuProcessLookupPTYs(vm-def-channels, vm-def-nchannels, paths, chardevfmt) 0) return -1; +/* For historical reasons, console[0] can be just an alias + * for serial[0]; That's why we need to update it as well */ Either you are starting a new sentence and need a full stop (serial[0]. That's), or you don't need a capital after semicolon (serial[0]; that's). Also, s/well/well./ wouldn't hurt. Fixed and pushed. Thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Fix segfault while redefining snapshots and allow redefinition of external snapshots
This series fixes two issues while redefining snapshots: 1) Segfault when disk alignment of the new snapshot failed (patch 3) 2) Failure to redefine external snapshots. A check was missing to align disks for external snapshots. (patch 4) Peter Krempa (4): snapshot: conf: Make virDomainSnapshotIsExternal more reusable snapshot: qemu: Separate logic blocks with newlines snapshot: qemu: Fix segfault and vanishing snapshots when redefining snapshot: qemu: Allow redefinition of external snapshots src/conf/snapshot_conf.c | 14 src/conf/snapshot_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 57 +++- 4 files changed, 54 insertions(+), 19 deletions(-) -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] snapshot: qemu: Allow redefinition of external snapshots
A redefinition of an external inactive snapshot/checkpoint wasn't possible without this change. --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4c7558d..3a52b47 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11410,7 +11410,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (def-dom) { if (def-state == VIR_DOMAIN_DISK_SNAPSHOT || -def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { +virDomainSnapshotDefIsExternal(def)) { align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; align_match = false; } -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] snapshot: conf: Make virDomainSnapshotIsExternal more reusable
Allow to use definition objects with this predicate function. --- src/conf/snapshot_conf.c | 14 ++ src/conf/snapshot_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 201c586..0c5b005 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -1058,17 +1058,23 @@ cleanup: bool -virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap) +virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def) { int i; -if (snap-def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) +if (def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) return true; -for (i = 0; i snap-def-ndisks; i++) { -if (snap-def-disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) +for (i = 0; i def-ndisks; i++) { +if (def-disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) return true; } return false; } + +bool +virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap) +{ +return virDomainSnapshotDefIsExternal(snap-def); +} diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index b0f8760..f1d5995 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -166,6 +166,7 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotPtr **snaps, unsigned int flags); +bool virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def); bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap); VIR_ENUM_DECL(virDomainSnapshotLocation) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 497d5d3..d063382 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1097,6 +1097,7 @@ virDomainSnapshotAlignDisks; virDomainSnapshotAssignDef; virDomainSnapshotDefFormat; virDomainSnapshotDefFree; +virDomainSnapshotDefIsExternal; virDomainSnapshotDefParseString; virDomainSnapshotDropParent; virDomainSnapshotFindByName; -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] snapshot: qemu: Separate logic blocks with newlines
--- src/qemu/qemu_driver.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b8bb36b..0883a56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11365,6 +11365,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; } + if (def-dom memcmp(def-dom-uuid, domain-uuid, VIR_UUID_BUFLEN)) { virReportError(VIR_ERR_INVALID_ARG, @@ -11372,6 +11373,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, def-name, uuidstr); goto cleanup; } + other = virDomainSnapshotFindByName(vm-snapshots, def-name); if (other) { if ((other-def-state == VIR_DOMAIN_RUNNING || @@ -11384,6 +11386,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, def-name); goto cleanup; } + if ((other-def-state == VIR_DOMAIN_DISK_SNAPSHOT) != (def-state == VIR_DOMAIN_DISK_SNAPSHOT)) { virReportError(VIR_ERR_INVALID_ARG, @@ -11392,6 +11395,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, def-name); goto cleanup; } + if (other-def-dom) { if (def-dom) { if (!virDomainDefCheckABIStability(other-def-dom, @@ -11403,10 +11407,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, other-def-dom = NULL; } } + if (other == vm-current_snapshot) { update_current = true; vm-current_snapshot = NULL; } + /* Drop and rebuild the parent relationship, but keep all * child relations by reusing snap. */ virDomainSnapshotDropParent(other); -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] snapshot: qemu: Fix segfault and vanishing snapshots when redefining
When the disk alignment check done while redefining an existing snapshot failed, the qemu driver attempted to free the existing snapshot. As in the cleanup path the definition of the snapshot wasn't assigned, the cleanup code dereferenced a NULL pointer. This patch changes the behavior on error paths while redefining snapshot in two ways: 1) On failure, modifications done on the snapshot definiton object are rolled back. 2) The previous definition of the data isn't freed until it's certain it won't be needed any more. This change avoids the segfault and additionaly the snapshot doesn't vanish if re-definiton fails for some reason. --- src/qemu/qemu_driver.c | 51 +++--- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0883a56..4c7558d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11408,6 +11408,24 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } } +if (def-dom) { +if (def-state == VIR_DOMAIN_DISK_SNAPSHOT || +def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { +align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; +align_match = false; +} + +if (virDomainSnapshotAlignDisks(def, align_location, +align_match) 0) { +/* revert stealing of the snapshot domain definition */ +if (def-dom !other-def-dom) { +other-def-dom = def-dom; +def-dom = NULL; +} +goto cleanup; +} +} + if (other == vm-current_snapshot) { update_current = true; vm-current_snapshot = NULL; @@ -11417,18 +11435,20 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * child relations by reusing snap. */ virDomainSnapshotDropParent(other); virDomainSnapshotDefFree(other-def); -other-def = NULL; +other-def = def; +def = NULL; snap = other; -} -if (def-dom) { -if (def-state == VIR_DOMAIN_DISK_SNAPSHOT || -def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { -align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; -align_match = false; +} else { +if (def-dom) { +if (def-state == VIR_DOMAIN_DISK_SNAPSHOT || +def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { +align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; +align_match = false; +} +if (virDomainSnapshotAlignDisks(def, align_location, +align_match) 0) +goto cleanup; } -if (virDomainSnapshotAlignDisks(def, align_location, -align_match) 0) -goto cleanup; } } else { /* Easiest way to clone inactive portion of vm-def is via @@ -11463,11 +11483,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; } -if (snap) -snap-def = def; -else if (!(snap = virDomainSnapshotAssignDef(vm-snapshots, def))) -goto cleanup; -def = NULL; +if (!snap) { +if (!(snap = virDomainSnapshotAssignDef(vm-snapshots, def))) +goto cleanup; + +def = NULL; +} if (update_current) snap-def-current = true; -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/8] virsh: Expose virDomainInterfacesAddresses
--- tools/virsh-domain.c | 104 +++ 1 file changed, 104 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f3da1d5..e6c09f0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8491,6 +8491,109 @@ cleanup: return ret; } +static const vshCmdInfo info_domifaddrs[] = { +{help, N_(Dump domain's IP addresses and other interfaces info)}, +{desc, N_(Dump domain's IP addresses and other interfaces info)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_domifaddrs[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{interface, VSH_OT_STRING, 0, N_(Limit selection just to one interface)}, +{method, VSH_OT_STRING, 0, N_(Use one method: default, agent, nwfilter)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdDomIfAddrs(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +bool ret = false; +unsigned int flags = 0; +const char *device = NULL; +const char *methodStr = NULL; +int method = VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT; +virDomainInterfacePtr *ifaces = NULL; +int i, j, ifacesCount = 0; + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +goto cleanup; + +if (vshCommandOptString(cmd, interface, device) 0) { +vshError(ctl, _(Unable to parse interface parameter)); +goto cleanup; +} + +if (vshCommandOptString(cmd, method, methodStr) 0) { +vshError(ctl, _(Unable to parse method parameter)); +goto cleanup; +} + +if (STREQ_NULLABLE(methodStr, default)) +method = VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT; +else if (STREQ_NULLABLE(methodStr, agent)) +method = VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT; +else if (STREQ_NULLABLE(methodStr, nwfilter)) +method = VIR_DOMAIN_INTERFACE_ADDRS_NWFILTER; +else if (methodStr) { +vshError(ctl, _(Unknown method: %s), methodStr); +goto cleanup; +} + +ifacesCount = virDomainInterfacesAddresses(dom, ifaces, method, flags); +if (ifacesCount 0) +goto cleanup; + +vshPrintExtra(ctl, %-10s %-17s%s\n%s\n, + _(Name), _(HW address), _(IP address), + ---); +for (i = 0; i ifacesCount; i++) { +virDomainInterfacePtr iface = ifaces[i]; +virBuffer buf = VIR_BUFFER_INITIALIZER; +const char *hwaddr = ; +const char *ipAddrStr = NULL; + +if (device STRNEQ(device, iface-name)) +continue; + +if (iface-hwaddr) +hwaddr = iface-hwaddr; + +for (j = 0; j iface-ip_addrs_count; j++) { +const char *type = ; +if (iface-ip_addrs[j].type == VIR_IP_ADDR_TYPE_IPV4) +type = inet ; +else if (iface-ip_addrs[j].type == VIR_IP_ADDR_TYPE_IPV6) +type = inet6 ; +if (j) +virBufferAddChar(buf, ' '); +virBufferAsprintf(buf, %s%s/%d, + type, + iface-ip_addrs[j].addr, + iface-ip_addrs[j].prefix); +} + +ipAddrStr = virBufferContentAndReset(buf); + +if (!ipAddrStr) +ipAddrStr = vshStrdup(ctl,); + +vshPrintExtra(ctl, %-10s %-17s%s\n, + iface-name, hwaddr, ipAddrStr); + +VIR_FREE(ipAddrStr); +} + +ret = true; +cleanup: +for (i = 0; i ifacesCount; i++) +virDomainInterfaceFree(ifaces[i]); +VIR_FREE(ifaces); +if (dom) +virDomainFree(dom); +return ret; +} + const vshCmdDef domManagementCmds[] = { {attach-device, cmdAttachDevice, opts_attach_device, info_attach_device, 0}, @@ -8527,6 +8630,7 @@ const vshCmdDef domManagementCmds[] = { {domhostname, cmdDomHostname, opts_domhostname, info_domhostname, 0}, {domid, cmdDomid, opts_domid, info_domid, 0}, {domif-setlink, cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, +{domifaddrs, cmdDomIfAddrs, opts_domifaddrs, info_domifaddrs, 0}, {domiftune, cmdDomIftune, opts_domiftune, info_domiftune, 0}, {domjobabort, cmdDomjobabort, opts_domjobabort, info_domjobabort, 0}, {domjobinfo, cmdDomjobinfo, opts_domjobinfo, info_domjobinfo, 0}, -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/8] qemu: Implement qemuDomainInterfacesAddresses
--- src/qemu/qemu_driver.c | 68 ++ 1 file changed, 68 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b8bb36b..662d956 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14520,6 +14520,73 @@ cleanup: return ret; } +static int +qemuDomainInterfacesAddresses(virDomainPtr dom, + virDomainInterfacePtr **ifaces, + unsigned int method, + unsigned int flags) +{ +virQEMUDriverPtr driver = dom-conn-privateData; +virDomainObjPtr vm; +int ret = -1; +qemuDomainObjPrivatePtr priv; + +virCheckFlags(0, -1); + +if (method != VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT +method != VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(Only guest agent method is supported for now)); +return -1; +} + +if (!(vm = qemuDomObjFromDomain(dom))) +goto cleanup; + +priv = vm-privateData; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(domain is not running)); +goto cleanup; +} + +if (!priv-agent) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, + _(QEMU guest agent is not configured)); +goto cleanup; +} + +if (priv-agentError) { +virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s, + _(QEMU guest agent is not + available due to an error)); +goto cleanup; +} + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(domain is not running)); +goto endjob; +} + +qemuDomainObjEnterAgent(driver, vm); +ret = qemuAgentGetInterfaces(priv-agent, ifaces); +qemuDomainObjExitAgent(driver, vm); + +endjob: +if (qemuDomainObjEndJob(driver, vm) == 0) +vm = NULL; + +cleanup: +if (vm) +virDomainObjUnlock(vm); +return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -14694,6 +14761,7 @@ static virDriver qemuDriver = { .nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */ .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */ .domainFSTrim = qemuDomainFSTrim, /* 1.0.1 */ +.domainInterfacesAddresses = qemuDomainInterfacesAddresses, /* 1.0.2 */ }; -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 8/8] python: create example for dumping domain IP addresses
--- examples/python/Makefile.am | 2 +- examples/python/README| 1 + examples/python/domipaddrs.py | 50 +++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100755 examples/python/domipaddrs.py diff --git a/examples/python/Makefile.am b/examples/python/Makefile.am index b04d126..b51f729 100644 --- a/examples/python/Makefile.am +++ b/examples/python/Makefile.am @@ -4,4 +4,4 @@ EXTRA_DIST=\ README \ consolecallback.py \ - dominfo.py domrestore.py domsave.py domstart.py esxlist.py + dominfo.py domrestore.py domsave.py domstart.py esxlist.py domipaddrs.py diff --git a/examples/python/README b/examples/python/README index f4db76c..d895740 100644 --- a/examples/python/README +++ b/examples/python/README @@ -10,6 +10,7 @@ domsave.py - save all running domU's into a directory domrestore.py - restore domU's from their saved files in a directory esxlist.py - list active domains of an VMware ESX host and print some info. also demonstrates how to use the libvirt.openAuth() method +domipaddrs.py - print domain interfaces among with their HW and IP addresses The XML files in this directory are examples of the XML format that libvirt expects, and will have to be adapted for your setup. They are only needed diff --git a/examples/python/domipaddrs.py b/examples/python/domipaddrs.py new file mode 100755 index 000..82c5774 --- /dev/null +++ b/examples/python/domipaddrs.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python +# domipaddrds - print IP addresses for running domain + +import libvirt +import sys + +def usage(): +print Usage: %s [URI] DOMAIN % sys.argv[0] +print Print IP addresses assigned to domain interfaces + +uri = None +name = None +args = len(sys.argv) + +if args == 2: +name = sys.argv[1] +elif args == 3: +uri = sys.argv[1] +name = sys.argv[2] +else: +usage() +sys.exit(2) + +conn = libvirt.openReadOnly(uri) +if conn == None: +print Unable to open connection to libvirt +sys.exit(1) + +try: +dom = conn.lookupByName(name) +except libvirt.libvirtError: +print Domain %s not found % name +sys.exit(0) + +ifaces = dom.interfacesAddresses(libvirt.VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT, 0) +if (ifaces == None): +print Failed to get domain interfaces +sys.exit(0) + +print {0:10} {1:17}{2}.format(Interface, HW address, IP Address) + +for (name, val) in ifaces.iteritems(): +print {0:10} {1:17}.format(name, val['hwaddr']), + +if (val['ip_addrs'] None): +print , +for addr in val['ip_addrs']: +print {0}/{1} .format(addr['addr'], addr['prefix']), + +print -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/8] remote: Implement virDomainInterfacesAddresses
--- daemon/remote.c | 140 +++ src/remote/remote_driver.c | 94 + src/remote/remote_protocol.x | 27 - src/remote_protocol-structs | 27 + 4 files changed, 287 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 8767c18..7363e2a 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4783,3 +4783,143 @@ no_memory: virReportOOMError(); return -1; } + +static int +remoteSerializeDomainInterfacesPtr(virDomainInterfacePtr *ifaces, + int ifacesCount, + remote_domain_interfaces_addresses_ret *ret) +{ +int i, j; + +if (!ifacesCount) +return 0; + +if (VIR_ALLOC_N(ret-ifaces.ifaces_val, ifacesCount) 0) { +virReportOOMError(); +return -1; +} + +ret-ifaces.ifaces_len = ifacesCount; + +for (i = 0; i ifacesCount; i++) { +virDomainInterfacePtr tmp = ifaces[i]; +remote_domain_interface *tmp_ret = (ret-ifaces.ifaces_val[i]); + +if (!(tmp_ret-name = strdup(tmp-name))) +goto no_memory; + +if (tmp-hwaddr) { +char **hwaddr_p = NULL; +if (VIR_ALLOC(hwaddr_p) 0) +goto no_memory; +*hwaddr_p = strdup(tmp-hwaddr); +if (!*hwaddr_p) { +VIR_FREE(hwaddr_p); +goto no_memory; +} + +tmp_ret-hwaddr = hwaddr_p; +} + +tmp_ret-flags = tmp-flags; + +if (!tmp-ip_addrs_count) +continue; + +if (VIR_ALLOC_N(tmp_ret-ip_addrs.ip_addrs_val, +tmp-ip_addrs_count) 0) +goto no_memory; + +tmp_ret-ip_addrs.ip_addrs_len = tmp-ip_addrs_count; + +for (j = 0; j tmp-ip_addrs_count; j++) { +virDomainIPAddress addr = tmp-ip_addrs[j]; +remote_domain_ip_addrs *addr_ret = (tmp_ret-ip_addrs.ip_addrs_val[j]); + +addr_ret-prefix = addr.prefix; +addr_ret-type = addr.type; + +if (addr.addr) { +char **addr_p = NULL; +if (VIR_ALLOC(addr_p) 0) +goto no_memory; +*addr_p = strdup(addr.addr); +if (!*addr_p) { +VIR_FREE(addr_p); +goto no_memory; +} +addr_ret-addr = addr_p; +} + +if (addr.dstaddr) { +char **addr_p = NULL; +if (VIR_ALLOC(addr_p) 0) +goto no_memory; +*addr_p = strdup(addr.dstaddr); +if (!*addr_p) { +VIR_FREE(addr_p); +goto no_memory; +} +addr_ret-dstaddr = addr_p; +} +} +} + +return 0; + +no_memory: +virReportOOMError(); +for (i = 0; i ret-ifaces.ifaces_len; i++) { +remote_domain_interface tmp_ret = ret-ifaces.ifaces_val[i]; + +VIR_FREE(tmp_ret.name); +VIR_FREE(tmp_ret.hwaddr); +for (j = 0; j tmp_ret.ip_addrs.ip_addrs_len; j++) { +VIR_FREE(tmp_ret.ip_addrs.ip_addrs_val[j].addr); +VIR_FREE(tmp_ret.ip_addrs.ip_addrs_val[j].dstaddr); +} +} +VIR_FREE(ret-ifaces.ifaces_val); + +return -1; +} + +static int +remoteDispatchDomainInterfacesAddresses(virNetServerPtr server ATTRIBUTE_UNUSED, +virNetServerClientPtr client, +virNetMessagePtr msg ATTRIBUTE_UNUSED, +virNetMessageErrorPtr rerr, + remote_domain_interfaces_addresses_args *args, +remote_domain_interfaces_addresses_ret *ret) +{ +int rv = -1; +virDomainPtr dom = NULL; +virDomainInterfacePtr *ifaces = NULL; +int ifacesCount = 0; +struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + +if (!(dom = get_nonnull_domain(priv-conn, args-domain))) +goto cleanup; + +ifacesCount = virDomainInterfacesAddresses(dom, ifaces, + args-method, + args-flags); +if (ifacesCount 0) +goto cleanup; + +if (remoteSerializeDomainInterfacesPtr(ifaces, ifacesCount, ret) 0) +goto cleanup; + +rv = 0; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); + +while (ifacesCount 0) +virDomainInterfaceFree(ifaces[--ifacesCount]); +VIR_FREE(ifaces); +return rv; +} diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ae861cc..7290f30 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5834,6 +5834,99 @@ done: return rv; }
[libvirt] [PATCH v2 3/8] qemu_agent: Implement 'guest-network-get-interfaces' command handling
This command returns an array of all guest interfaces among with their IP and HW addresses. --- src/qemu/qemu_agent.c | 171 ++ src/qemu/qemu_agent.h | 2 + 2 files changed, 173 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bb421bd..409ba04 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1474,3 +1474,174 @@ qemuAgentFSTrim(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +static int +getInterfaces(virJSONValuePtr reply, + virDomainInterfacePtr **ifaces) +{ +int ret = -1; +int i, size = 0; +virJSONValuePtr replyArray = NULL; + +if (!(replyArray = virJSONValueObjectGet(reply, return))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu agent did not provide 'return' object)); +goto cleanup; +} + +if ((size = virJSONValueArraySize(replyArray)) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu agent did not provide any interface)); +goto cleanup; +} + +if (size VIR_ALLOC_N(*ifaces, size * sizeof(virDomainInterfacePtr)) 0) { +virReportOOMError(); +goto cleanup; +} + +for (i = 0; i size; i++) { +virJSONValuePtr jsonIface = virJSONValueArrayGet(replyArray, i); +virDomainInterfacePtr tmpIface = NULL; +virJSONValuePtr jsonIpAddrArr = NULL; +int j, jsonIpAddrArrSize = 0; +const char *name, *hwaddr; + +if (VIR_ALLOC(tmpIface) 0) { +virReportOOMError(); +goto cleanup; +} + +(*ifaces)[i] = tmpIface; +/* should not happen, but doesn't hurt to check */ +if (!jsonIface) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Something went really wrong while processing + guest agent reply)); +goto cleanup; +} + +name = virJSONValueObjectGetString(jsonIface, name); +if (!name) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu agent did not provide 'name' object)); +goto cleanup; +} + +if (!(tmpIface-name = strdup(name))) { +virReportOOMError(); +goto cleanup; +} + +/* hwaddr might be omitted */ +hwaddr = virJSONValueObjectGetString(jsonIface, hardware-address); +if (hwaddr !(tmpIface-hwaddr = strdup(hwaddr))) { +virReportOOMError(); +goto cleanup; +} + +/* as well as ip-addresses */ +jsonIpAddrArr = virJSONValueObjectGet(jsonIface, ip-addresses); +if (!jsonIpAddrArr) +continue; + +if ((jsonIpAddrArrSize = virJSONValueArraySize(jsonIpAddrArr)) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu agent provided malformed + ip-addresses field)); +goto cleanup; +} + +if (VIR_ALLOC_N(tmpIface-ip_addrs, jsonIpAddrArrSize) 0) { +virReportOOMError(); +goto cleanup; +} +tmpIface-ip_addrs_count = jsonIpAddrArrSize; + +for (j = 0; j jsonIpAddrArrSize; j++) { +virJSONValuePtr jsonIpAddr = virJSONValueArrayGet(jsonIpAddrArr, j); +virDomainIPAddressPtr tmpIpAddr = (tmpIface-ip_addrs[j]); +const char *ipAddr, *ipAddrType; + +if (!(ipAddr = virJSONValueObjectGetString(jsonIpAddr, + ip-address))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu-agent didn't provided + an ip-address field)); +goto cleanup; +} + +if (!(tmpIpAddr-addr = strdup(ipAddr))) { +virReportOOMError(); +goto cleanup; +} + +if (!(ipAddrType = virJSONValueObjectGetString(jsonIpAddr, + ip-address-type))) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(qemu-agent didn't provided + an ip-address-type field)); +goto cleanup; +} + +if (STREQ(ipAddrType, ipv4)) +tmpIpAddr-type = VIR_IP_ADDR_TYPE_IPV4; +else if (STREQ(ipAddrType, ipv6)) +tmpIpAddr-type = VIR_IP_ADDR_TYPE_IPV6; +else { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(qemu agent provided unknown + ip-address-type '%s'), + ipAddrType); +goto cleanup; +} + +if (virJSONValueObjectGetNumberInt(jsonIpAddr, prefix, +
[libvirt] [PATCH v2 7/8] python: Expose virDomainInterfacesAddresses
--- python/libvirt-override-api.xml | 7 +++ python/libvirt-override.c | 120 2 files changed, 127 insertions(+) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index a0e0496..bea1a59 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -548,5 +548,12 @@ arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/ arg name='flags' type='int' info='unused, pass 0'/ /function +function name='virDomainInterfacesAddresses' file='python' + infoGet domain's interfaces among with their IP and HW addresses/info + return type='virDomainInterfacePtr' info='array of @domain interfaces'/ + arg name='domain' type='virDomainPtr' info='domain object'/ + arg name='method' type='unsigned int' info='which method use, one of virDomainInterfacesAddressesMethod'/ + arg name='flags' type='unsigned int' info='extra flags, not used yet, so callers should always pass 0'/ +/function /symbols /api diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 91e82c6..c8365d4 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -6588,6 +6588,125 @@ error: goto cleanup; } +static PyObject * +libvirt_virDomainInterfacesAddresses(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ +PyObject *py_retval; +virDomainPtr domain; +PyObject *pyobj_domain; +unsigned int method, flags; +virDomainInterfacePtr *ifaces = NULL; +int j, i, i_retval = 0; +bool full_free = true; + +if (!PyArg_ParseTuple(args, (char *) Oii:virDomainInterfacesAddresses, + pyobj_domain, method, flags)) +return NULL; + +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; +i_retval = virDomainInterfacesAddresses(domain, ifaces, method, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (i_retval 0) { +py_retval = VIR_PY_NONE; +goto cleanup; +} + +if (!(py_retval = PyDict_New())) +goto no_memory; + +for (i = 0; i i_retval; i++) { +virDomainInterfacePtr iface = ifaces[i]; +PyObject *pyIPAddrs = NULL; +PyObject *pyIface = NULL; + +if (!(pyIface = PyDict_New())) +goto no_memory; + +if (iface-ip_addrs_count) { +if (!(pyIPAddrs = PyList_New(iface-ip_addrs_count))) { +Py_DECREF(pyIface); +goto no_memory; +} +} else { +pyIPAddrs = VIR_PY_NONE; +} + +for (j = 0; j iface-ip_addrs_count; j++) { +virDomainIPAddress addr = iface-ip_addrs[j]; +PyObject *pyAddr = PyDict_New(); +const char *type = NULL; + +if (!pyAddr) { +{ Py_DECREF(pyIface); } +{ Py_DECREF(pyIPAddrs); } +goto no_memory; +} + +switch (addr.type) { +case VIR_IP_ADDR_TYPE_IPV4: +type = ipv4; +break; +case VIR_IP_ADDR_TYPE_IPV6: +type = ipv6; +break; +default: +type = unknown; +break; +} + +PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(addr), + PyString_FromString(addr.addr)); +PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(prefix), + libvirt_intWrap(addr.prefix)); +PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(type), + PyString_FromString(type)); +PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(dstaddr), + libvirt_charPtrWrap(addr.dstaddr)); +PyList_SetItem(pyIPAddrs, j, pyAddr); +} + +PyDict_SetItem(pyIface, libvirt_constcharPtrWrap(ip_addrs), + pyIPAddrs); +PyDict_SetItem(pyIface, libvirt_constcharPtrWrap(hwaddr), + libvirt_charPtrWrap(iface-hwaddr)); +PyDict_SetItem(pyIface, libvirt_constcharPtrWrap(flags), + libvirt_intWrap(iface-flags)); + +PyDict_SetItem(py_retval, libvirt_constcharPtrWrap(iface-name), + pyIface); +} + +full_free = false; + + +cleanup: +for (i = 0; i i_retval; i++) { +/* We don't want to free values we've just + * shared with python variables unless + * there was an error and hence we are + * returning PY_NONE or equivalent */ +if (full_free) { +VIR_FREE(ifaces[i]-name); +VIR_FREE(ifaces[i]-hwaddr); +for (j = 0; j ifaces[i]-ip_addrs_count; j++) { +VIR_FREE(ifaces[i]-ip_addrs[j].addr); +VIR_FREE(ifaces[i]-ip_addrs[j].dstaddr); +} +} +
[libvirt] [PATCH v2 0/8] Dump domain's IP addresses
It's been a while since I tried get this in. I've reworked the patches, the exposed API, and start new round of reviews. Moreover, during work I've come to point, where extending qemu guest agent seemed wise: http://lists.nongnu.org/archive/html/qemu-devel/2012-12/msg03264.html Therefore I am introducing 'flags' field, which can contain some boolean values we don't have yet, e.g. IFF_UP, IFF_PROMISC, etc. Re: 'dstaddr' field in struct _virDomainInterfaceIPAddress; It's basically a join of: union { char *dstaddr; /* for IFF_POINTOPOINT interface */ char *broadaddr; /* for IFF_BROADCAST interface */ } Since an interface cannot has both flags set (see man 3 getifaddrs) I've joined both into one 'char *dstaddr'. I know it is not mnemonic as the union, so I left it for discussion. I can change it if you want to. diff to v1: -don't return array of objects, but array of pointer to objects instead Michal Privoznik (8): Introduce virDomainInterfacesAddresses API Introduce virDomainInterfaceFree API qemu_agent: Implement 'guest-network-get-interfaces' command handling qemu: Implement qemuDomainInterfacesAddresses virsh: Expose virDomainInterfacesAddresses remote: Implement virDomainInterfacesAddresses python: Expose virDomainInterfacesAddresses python: create example for dumping domain IP addresses daemon/remote.c | 140 examples/python/Makefile.am | 2 +- examples/python/README | 1 + examples/python/domipaddrs.py | 50 include/libvirt/libvirt.h.in| 51 python/generator.py | 3 + python/libvirt-override-api.xml | 7 ++ python/libvirt-override.c | 120 src/driver.h| 6 ++ src/libvirt.c | 107 + src/libvirt_public.syms | 7 ++ src/qemu/qemu_agent.c | 171 src/qemu/qemu_agent.h | 2 + src/qemu/qemu_driver.c | 68 src/remote/remote_driver.c | 94 ++ src/remote/remote_protocol.x| 27 ++- src/remote_protocol-structs | 27 +++ tools/virsh-domain.c| 104 18 files changed, 985 insertions(+), 2 deletions(-) create mode 100755 examples/python/domipaddrs.py -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/8] Introduce virDomainInterfacesAddresses API
This API returns dynamically allocated array of IP addresses for all domain interfaces. --- include/libvirt/libvirt.h.in | 49 ++ python/generator.py | 2 ++ src/driver.h | 6 src/libvirt.c| 71 src/libvirt_public.syms | 6 5 files changed, 134 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index cc3fe13..3ee6688 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4563,6 +4563,55 @@ int virDomainFSTrim(virDomainPtr dom, unsigned long long minimum, unsigned int flags); +typedef enum { +VIR_INTERFACE_BROADCAST = (1 0), /* Broadcast address valid. */ +VIR_INTERFACE_PPP = (1 1), /* Interface is point-to-point. */ +/* we can add new flags here */ +} virDomainInterfaceType; + +typedef enum { +VIR_IP_ADDR_TYPE_IPV4, +VIR_IP_ADDR_TYPE_IPV6, + +#ifdef VIR_ENUM_SENTINELS +VIR_IP_ADDR_TYPE_LAST +#endif +} virIPAddrType; + + +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress; +typedef virDomainIPAddress *virDomainIPAddressPtr; +struct _virDomainInterfaceIPAddress { +int type; /* virIPAddrType */ +char *addr; /* IP address */ +int prefix; /* IP address prefix */ +char *dstaddr; /* Broadcast address (if @flags VIR_INTERFACE_BROADCAST), + PPP destination address (if @flags VIR_INTERFACE_PPP) */ +}; + +typedef struct _virDomainInterface virDomainInterface; +typedef virDomainInterface *virDomainInterfacePtr; +struct _virDomainInterface { +char *name; /* interface name */ +unsigned int flags; /* virDomainInterfaceType */ +char *hwaddr; /* hardware address */ +unsigned int ip_addrs_count;/* number of items in @ip_addr */ +virDomainIPAddressPtr ip_addrs; /* array of IP addresses */ +}; + +typedef enum { +VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT = 0, /* hypervisor choice */ +VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT = 1, /* use guest agent */ +VIR_DOMAIN_INTERFACE_ADDRS_NWFILTER = 2, /* use nwfilter learning code */ +/* etc ... */ +} virDomainInterfacesAddressesMethod; + + +int virDomainInterfacesAddresses(virDomainPtr domain, + virDomainInterfacePtr **ifaces, + unsigned int method, + unsigned int flags); + /** * virSchedParameterType: * diff --git a/python/generator.py b/python/generator.py index bae4edc..ee39e51 100755 --- a/python/generator.py +++ b/python/generator.py @@ -243,6 +243,7 @@ skipped_types = { 'virEventHandleCallback': No function types in python, 'virEventTimeoutCallback': No function types in python, 'virDomainBlockJobInfoPtr': Not implemented yet, + 'virDomainInterfacePtr': Not implemented yet, } ### @@ -428,6 +429,7 @@ skip_impl = ( 'virNodeGetMemoryParameters', 'virNodeSetMemoryParameters', 'virNodeGetCPUMap', +'virDomainInterfacesAddresses', ) qemu_skip_impl = ( diff --git a/src/driver.h b/src/driver.h index 64d652f..dc93ffb 100644 --- a/src/driver.h +++ b/src/driver.h @@ -914,6 +914,11 @@ typedef int const char *mountPoint, unsigned long long minimum, unsigned int flags); +typedef int +(*virDrvDomainInterfacesAddresses)(virDomainPtr domain, + virDomainInterfacePtr **ifaces, + unsigned int method, + unsigned int flags); /** * _virDriver: @@ -1107,6 +1112,7 @@ struct _virDriver { virDrvNodeGetCPUMap nodeGetCPUMap; virDrvDomainFSTrim domainFSTrim; virDrvDomainSendProcessSignal domainSendProcessSignal; +virDrvDomainInterfacesAddresses domainInterfacesAddresses; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index bf674d1..6f0de36 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20424,3 +20424,74 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainInterfacesAddresses: + * @domain: domain object + * @ifaces: array of @domain interfaces + * @method: which method use, one of virDomainInterfacesAddressesMethod + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Return an array of interfaces presented in given @domain among with + * their IP and HW addresses. Single interface can have multiple or even + * none IP address. + * + * This API dynamically allocates the virDomainInterfacePtr struct based on + * how much interfaces domain has, usually there's 1:1 correlation between + * interfaces seen from the host and interfaces seen from the
[libvirt] [PATCH v2 2/8] Introduce virDomainInterfaceFree API
This is just a free function for array of virDomainInterfacePtr as returned by virDomainInterfacesAddresses API. --- include/libvirt/libvirt.h.in | 2 ++ python/generator.py | 1 + src/libvirt.c| 36 src/libvirt_public.syms | 1 + 4 files changed, 40 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 3ee6688..4199cd3 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4612,6 +4612,8 @@ int virDomainInterfacesAddresses(virDomainPtr domain, unsigned int method, unsigned int flags); +void virDomainInterfaceFree(virDomainInterfacePtr iface); + /** * virSchedParameterType: * diff --git a/python/generator.py b/python/generator.py index ee39e51..e0a6f10 100755 --- a/python/generator.py +++ b/python/generator.py @@ -430,6 +430,7 @@ skip_impl = ( 'virNodeSetMemoryParameters', 'virNodeGetCPUMap', 'virDomainInterfacesAddresses', +'virDomainInterfaceFree', ) qemu_skip_impl = ( diff --git a/src/libvirt.c b/src/libvirt.c index 6f0de36..6f4ce60 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20495,3 +20495,39 @@ error: virDispatchError(domain-conn); return -1; } + +/** + * virDomainInterfaceFree: + * @iface: array of interfaces + * + * Free an interface as returned by virDomainInterfacesAddresses. + * The @ifaces pointer is freed and should not be used thereafter. + * Basic usage is: + * + * virDomainInterfacePtr *iface = NULL; + * int i, j; + * + * i = virDomainInterfacesAddresses(dom, iface, method, flags); + * + * Do something here ...; + * + * for (j = 0; j i; j ++) + * virDomainInterfaceFree(iface[j]); + * free(iface); + */ +void +virDomainInterfaceFree(virDomainInterfacePtr iface) +{ +int i; +VIR_DEBUG(iface=%p, iface); + + +VIR_FREE(iface-name); +VIR_FREE(iface-hwaddr); +for (i = 0; i iface-ip_addrs_count; i++) { +VIR_FREE(iface-ip_addrs[i].addr); +VIR_FREE(iface-ip_addrs[i].dstaddr); +} +VIR_FREE(iface-ip_addrs); +VIR_FREE(iface); +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 8e7c5d2..a401363 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -583,6 +583,7 @@ LIBVIRT_1.0.1 { LIBVIRT_1.0.2 { global: virDomainInterfacesAddresses; +virDomainInterfaceFree; } LIBVIRT_1.0.1; -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu
On 12/26/12 02:00, liguang wrote: @@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, * When QEMU grows support for 1 PCI domain, then pci.0 change * to pciNN.0 where NN is the domain number */ -if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) +if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) { +virBufferAsprintf(buf, ,bus=pci-bridge%d, info-addr.pci.bus); +} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) { virBufferAsprintf(buf, ,bus=pci.0); Is there any way (or plan) to use more pci buses with QEMU other than with the pci bridges? If not, we could just name the bridges pci.%d. (If we index the bridges from 1). -else +} else { virBufferAsprintf(buf, ,bus=pci); +} if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) virBufferAddLit(buf, ,multifunction=on); else if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF) @@ -3455,6 +3460,32 @@ error: return NULL; } +char * +qemuBuildPCIbridgeDevStr(virDomainPCIbridgeDefPtr dev, + qemuCapsPtr caps, int idx) +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; + +virBufferAsprintf(buf, pci-bridge,chassis_nr=1); The chassis number has to be unique for each bridge. + +if ((dev-type != VIR_DOMAIN_PCIBRIDGE_TYPE_ROOT) +(qemuBuildDeviceAddressStr(buf, dev-info, caps) 0)) +goto error; +else +virBufferAsprintf(buf, ,id=pci-bridge%d , idx); + +if (virBufferError(buf)) { +virReportOOMError(); +goto error; +} + +return virBufferContentAndReset(buf); + +error: +virBufferFreeAndReset(buf); +return NULL; + +} -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php][PATCH 00/20] compilation under OS X and various
Hi Tugdual! Thanks for the patch series. I've applied and tested it and it's working great! Thanks again for the series. I've noted your major contribution in the AUTHORS file with big thanks going to you :-) Michal On 01/03/2013 10:34 AM, Tugdual Saunier wrote: These patches are the result of my work to get libvirt-php compiled under OS X. As I suceeded to compiled it, I found some bugs/warnings that are also fixed in the following commits. The most important is probably the vnc_get_bitmap and vnc_refresh_screen now working. Tugdual Saunier (20): Fixed compilation under OS X fixed a malloc_error_break in doc generation fixed wrong xpath in domain type retrieving fixed PHP warnings in uuid generation fixed warnings fixed resource counter changes in libvirt_list_active_domains init array only if needed in libvirt_list_inactive_domains fixed wrong maxsize type in libvirt_logfile_set fixed wrong types in libvirt_check_version fixed wrong byte_order determination under other OS by improving it fixed wrong test on virConnectClose return value fixed vnc_get_bitmap Fixed infinite loop in vnc_refresh_screen fixed last warnings fixed doc generation under OSX Made tests more universal by using libvirt test driver reorder tests by dependency improved test-install.phpt, no more need of a random name fixed some segfault in test-domain-snapshot.phpt fixed wrong doc alignment for libvirt_domain_managedsave configure.ac | 10 + docs/Makefile.am | 14 +- src/Makefile.am | 5 +- src/libvirt-php.c| 330 +-- src/libvirt-php.h| 16 +- src/vncfunc.c| 25 +- tests/data/example-no-disk-and-media.xml | 3 +- tests/functions.phpt | 8 + tests/php.ini| 2 + tests/runtests.sh| 11 +- tests/test-conn-limit.phpt | 2 +- tests/test-connect.phpt | 40 +--- tests/test-domain-create-and-coredump.phpt | 2 +- tests/test-domain-create-and-get-xpath.phpt | 4 +- tests/test-domain-create.phpt| 2 +- tests/test-domain-define-create-destroy.phpt | 2 +- tests/test-domain-define-undefine.phpt | 2 +- tests/test-domain-snapshot.phpt | 14 +- tests/test-get-emulator.phpt | 10 +- tests/test-install.phpt | 14 +- tests/test-logging.phpt | 6 +- tools/generate-api-docs.c| 1 + 22 files changed, 260 insertions(+), 263 deletions(-) create mode 100644 tests/data/test-libvirt-php.img create mode 100644 tests/php.ini -- 1.8.0.1 -- Michal Novotny minov...@redhat.com, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Split out parts of qemu_driver.c into separate files
qemu_driver.c is starting to grow out of proportions. This series creates two(actualy four) files to split out some code from the main file. 1) qemu_util.c - various qemu-related tools and common code (~0.5k lines) 2) qemu_snapshot.c - snapshot related code (~1.7k lines) Apart from reformating function headers, no changes were made to the code. Peter Krempa (2): qemu: Split out various utility functions to qemu_util.c qemu: Split out snapshot related functions to qemu_snapshot.c po/POTFILES.in |2 + src/Makefile.am |2 + src/qemu/qemu_driver.c | 2191 +- src/qemu/qemu_snapshot.c | 1752 src/qemu/qemu_snapshot.h | 38 + src/qemu/qemu_util.c | 486 ++ src/qemu/qemu_util.h | 111 +++ 7 files changed, 2404 insertions(+), 2178 deletions(-) create mode 100644 src/qemu/qemu_snapshot.c create mode 100644 src/qemu/qemu_snapshot.h create mode 100644 src/qemu/qemu_util.c create mode 100644 src/qemu/qemu_util.h -- 1.8.0.2 e -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c
--- po/POTFILES.in | 1 + src/Makefile.am| 1 + src/qemu/qemu_driver.c | 492 ++--- src/qemu/qemu_util.c | 486 src/qemu/qemu_util.h | 111 +++ 5 files changed, 611 insertions(+), 480 deletions(-) create mode 100644 src/qemu/qemu_util.c create mode 100644 src/qemu/qemu_util.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 4d94799..4edacfa 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -101,6 +101,7 @@ src/qemu/qemu_monitor.c src/qemu/qemu_monitor_json.c src/qemu/qemu_monitor_text.c src/qemu/qemu_process.c +src/qemu/qemu_util.c src/remote/remote_client_bodies.h src/remote/remote_driver.c src/rpc/virkeepalive.c diff --git a/src/Makefile.am b/src/Makefile.am index f7a9b91..f76a2ea 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -515,6 +515,7 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_conf.c qemu/qemu_conf.h \ qemu/qemu_process.c qemu/qemu_process.h \ qemu/qemu_migration.c qemu/qemu_migration.h \ + qemu/qemu_util.c qemu/qemu_util.h \ qemu/qemu_monitor.c qemu/qemu_monitor.h \ qemu/qemu_monitor_text.c\ qemu/qemu_monitor_text.h\ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3a52b47..a9c03b6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1,7 +1,7 @@ /* * qemu_driver.c: core driver methods for managing qemu guests * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -57,6 +57,7 @@ #include qemu_bridge_filter.h #include qemu_process.h #include qemu_migration.h +#include qemu_util.h #include virerror.h #include virlog.h @@ -186,97 +187,6 @@ struct qemuAutostartData { virConnectPtr conn; }; -/** - * qemuDomObjFromDomainDriver: - * @domain: Domain pointer that has to be looked up - * @drv: Pointer to virQEMUDriverPtr to return the driver object - * - * This function looks up @domain in the domain list and returns the - * appropriate virDomainObjPtr. On successful lookup, both driver and domain - * object are returned locked. - * - * Returns the domain object if it's found and the driver. Both are locked. - * In case of failure NULL is returned and the driver isn't locked. - */ -static virDomainObjPtr -qemuDomObjFromDomainDriver(virDomainPtr domain, virQEMUDriverPtr *drv) -{ -virQEMUDriverPtr driver = domain-conn-privateData; -virDomainObjPtr vm; -char uuidstr[VIR_UUID_STRING_BUFLEN]; - -qemuDriverLock(driver); -vm = virDomainFindByUUID(driver-domains, domain-uuid); -if (!vm) { -virUUIDFormat(domain-uuid, uuidstr); -virReportError(VIR_ERR_NO_DOMAIN, - _(no domain with matching uuid '%s'), uuidstr); -qemuDriverUnlock(driver); -*drv = NULL; -return NULL; -} - -*drv = driver; -return vm; -} - -/** - * qemuDomObjFromDomain: - * @domain: Domain pointer that has to be looked up - * - * This function looks up @domain and returns the appropriate - * virDomainObjPtr. The driver is unlocked after the call. - * - * Returns the domain object which is locked on success, NULL - * otherwise. The driver remains unlocked after the call. - */ -static virDomainObjPtr -qemuDomObjFromDomain(virDomainPtr domain) -{ -virDomainObjPtr vm; -virQEMUDriverPtr driver; - -if (!(vm = qemuDomObjFromDomainDriver(domain, driver))) -return NULL; - -qemuDriverUnlock(driver); - -return vm; -} - -/* Looks up the domain object from snapshot and unlocks the driver. The - * returned domain object is locked and the caller is responsible for - * unlocking it */ -static virDomainObjPtr -qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot) -{ -return qemuDomObjFromDomain(snapshot-domain); -} - - -/* Looks up snapshot object from VM and name */ -static virDomainSnapshotObjPtr -qemuSnapObjFromName(virDomainObjPtr vm, -const char *name) -{ -virDomainSnapshotObjPtr snap = NULL; -snap = virDomainSnapshotFindByName(vm-snapshots, name); -if (!snap) -virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _(no domain snapshot with matching name '%s'), - name); - -return snap; -} - - -/* Looks up snapshot object from VM and snapshotPtr */ -static virDomainSnapshotObjPtr -qemuSnapObjFromSnapshot(virDomainObjPtr vm, -virDomainSnapshotPtr snapshot) -{ -return qemuSnapObjFromName(vm, snapshot-name); -} static void qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, @@
[libvirt] [PATCH] fwrite: silence __wur without using inline
The libvirt folks reported[1] a link error of multiple rpl_fwrite definitions that hits only when optimization and FORTIFY_SOURCE are both enabled, due to improper use of inline. But since that particular use of rpl_fwrite exists only to work around a spurious gcc warning on some versions of glibc, we can use gcc extensions to acheive the same effect without using inline. This approach copies the method used in ignore-value.h. [1] https://lists.gnu.org/archive/html/bug-gnulib/2013-01/msg00014.html * lib/stdio.in.h (fwrite): Limit warn_unused_result workaround to just gcc, and in a way that avoids inline issues. * modules/stdio (Depends-on): Drop extern-inline. Signed-off-by: Eric Blake ebl...@redhat.com --- ChangeLog | 7 +++ lib/stdio.in.h | 23 +-- modules/stdio | 1 - 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index 73c186d..0bae990 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2013-01-03 Eric Blake ebl...@redhat.com + + fwrite: silence __wur without using inline + * lib/stdio.in.h (fwrite): Limit warn_unused_result workaround to + just gcc, and in a way that avoids inline issues. + * modules/stdio (Depends-on): Drop extern-inline. + 2013-01-03 Jim Meyering j...@meyering.net update-copyright: avoid copyright notice date corruption diff --git a/lib/stdio.in.h b/lib/stdio.in.h index ec39c7d..0f20171 100644 --- a/lib/stdio.in.h +++ b/lib/stdio.in.h @@ -46,11 +46,6 @@ #ifndef _@GUARD_PREFIX@_STDIO_H #define _@GUARD_PREFIX@_STDIO_H -_GL_INLINE_HEADER_BEGIN -#ifndef _GL_STDIO_INLINE -# define _GL_STDIO_INLINE _GL_INLINE -#endif - /* Get va_list. Needed on many systems, including glibc 2.8. */ #include stdarg.h @@ -583,18 +578,12 @@ _GL_CXXALIAS_SYS (fwrite, size_t, /* Work around glibc bug 11959 http://sources.redhat.com/bugzilla/show_bug.cgi?id=11959, which sometimes causes an unwanted diagnostic for fwrite calls. - This affects only function declaration attributes, so it's not - needed for C++. */ -# if !defined __cplusplus 0 __USE_FORTIFY_LEVEL -_GL_STDIO_INLINE size_t _GL_ARG_NONNULL ((1, 4)) -rpl_fwrite (const void *ptr, size_t s, size_t n, FILE *stream) -{ - size_t r = fwrite (ptr, s, n, stream); - (void) r; - return r; -} + This affects only function declaration attributes under certain + versions of gcc, and is not needed for C++. */ +# if !defined __cplusplus 0 __USE_FORTIFY_LEVEL \ + 3 (__GNUC__ + (4 = __GNUC_MINOR__)) # undef fwrite -# define fwrite rpl_fwrite +# define fwrite(a, b, c, d) ({size_t __r = fwrite (a, b, c, d); __r; }) # endif # endif _GL_CXXALIASWARN (fwrite); @@ -1338,8 +1327,6 @@ _GL_WARN_ON_USE (vsprintf, vsprintf is not always POSIX compliant - POSIX compliance); #endif -_GL_INLINE_HEADER_END - #endif /* _@GUARD_PREFIX@_STDIO_H */ #endif /* _@GUARD_PREFIX@_STDIO_H */ #endif diff --git a/modules/stdio b/modules/stdio index c33ad31..54189dc 100644 --- a/modules/stdio +++ b/modules/stdio @@ -7,7 +7,6 @@ lib/stdio.in.h m4/stdio_h.m4 Depends-on: -extern-inline include_next snippet/arg-nonnull snippet/c++defs -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix build with optimization enabled
Build failure detected by Jenkins autobuilder: http://honk.sigxcpu.org:8001/job/libvirt-build/472/changes * .gnulib: Update to latest, for stdio.h rpl_fwrite fix. --- Pushing under the build-breaker rule. I managed to reproduce the link failure on a RHEL 6 box with CFLAGS='-g -O2', and also that this gnulib update made it go away. * .gnulib 964bbc2...61c7b1e (2): fwrite: silence __wur without using inline update-copyright: avoid copyright notice date corruption .gnulib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gnulib b/.gnulib index 964bbc2..61c7b1e 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 964bbc2d419584e93fe629ddbc40595612f62083 +Subproject commit 61c7b1e32e11e9e40b4d59ab888a807620befcd3 -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirtd segfault
On 01/02/2013 09:45 AM, Scott Sullivan wrote: On 12/29/2012 04:09 AM, Michal Privoznik wrote: On 28.12.2012 20:23, Scott Sullivan wrote: snip/ I have just now received another SIGSEGV, with your patch applied. Here's the info from the GDB session: Detaching after fork from child process 11266. 2012-12-28 18:56:53.261+: 29943: error : qemuMonitorIO:614 : internal error End of file from monitor Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffec0cd700 (LWP 29955)] qemuDomainObjBeginJobInternal (driver=0x7fffe4013520, driver_locked=true, obj=0x7fff7801fc80, job=QEMU_JOB_DESTROY, asyncJob=QEMU_ASYNC_JOB_NONE) at qemu/qemu_domain.c:780 780 priv-jobs_queued++; (gdb) bt #0 qemuDomainObjBeginJobInternal (driver=0x7fffe4013520, driver_locked=true, obj=0x7fff7801fc80, job=QEMU_JOB_DESTROY, asyncJob=QEMU_ASYNC_JOB_NONE) at qemu/qemu_domain.c:780 #1 0x7fffea599f46 in qemuDomainDestroyFlags (dom=value optimized out, flags=value optimized out) at qemu/qemu_driver.c:2189 #2 0x77a83587 in virDomainDestroy (domain=0x7fffe414a510) at libvirt.c:2215 #3 0x004296e2 in remoteDispatchDomainDestroy (server=value optimized out, client=value optimized out, msg=value optimized out, rerr=0x7fffec0ccbc0, args=value optimized out, ret=value optimized out) at remote_dispatch.h:1277 #4 remoteDispatchDomainDestroyHelper (server=value optimized out, client=value optimized out, msg=value optimized out, rerr=0x7fffec0ccbc0, args=value optimized out, ret=value optimized out) at remote_dispatch.h:1255 #5 0x77ad0d02 in virNetServerProgramDispatchCall (prog=0x6814d0, server=0x678df0, client=0x693a80, msg=0x6986d0) at rpc/virnetserverprogram.c:431 #6 virNetServerProgramDispatch (prog=0x6814d0, server=0x678df0, client=0x693a80, msg=0x6986d0) at rpc/virnetserverprogram.c:304 #7 0x77aceaa6 in virNetServerProcessMsg (srv=value optimized out, client=0x693a80, prog=value optimized out, msg=0x6986d0) at rpc/virnetserver.c:173 #8 0x77acf5e3 in virNetServerHandleJob (jobOpaque=value optimized out, opaque=0x678df0) at rpc/virnetserver.c:194 #9 0x779e8fdc in virThreadPoolWorker (opaque=value optimized out) at util/threadpool.c:144 #10 0x779e88c9 in virThreadHelper (data=value optimized out) at util/threads-pthread.c:161 #11 0x00300a2077f1 in start_thread () from /lib64/libpthread.so.0 #12 0x003009ae570d in clone () from /lib64/libc.so.6 (gdb) This means, even though we successfully incremented reference counter on virDomainObjPtr object, somebody free()d it anyway (well, the privateData at least). Looks like a locking/concurrent access issue to me then. Unfortunately, I don't have any suggestions yet, as the domain object is supposed to be locked when entering the qemuDomainObjBeginJobInternal() function so it shouldn't get free()d meanwhile. Michal Michal, I have a faster way to reproduce the crash (~10 minutes). Continue to read for new (easier) steps. This test was done with the standard v1.0.0 libvirtd code source, with no other patches applied. snip Not sure how much this helps, but in my testing I have found this issue was introduced with v0.9.12. I cannot reproduce this issue under v0.9.11.X or older. Comparing src/qemu/qemu_domain.c between v0.9.11.X and v0.9.12 I see numerous changes to the code related to locking/concurrency. For instance, the introduction of qemuDomainTrackJob as one large difference I see. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c
On 01/03/2013 09:45 AM, Peter Krempa wrote: --- po/POTFILES.in | 1 + src/Makefile.am| 1 + src/qemu/qemu_driver.c | 492 ++--- src/qemu/qemu_util.c | 486 src/qemu/qemu_util.h | 111 +++ 5 files changed, 611 insertions(+), 480 deletions(-) create mode 100644 src/qemu/qemu_util.c create mode 100644 src/qemu/qemu_util.h + +static inline void +bswap_header(virQEMUSaveHeaderPtr hdr) { It looks weird that you moved this function, but only to later in the same file rather than into a new file, and also weird that this function doesn't follow naming conventions. I would leave it untouched in this patch, and then do a followup patch that inlines the body of this function into its lone caller (qemuDomainSaveImageOpen); which will get rid of the bad naming at the same time. +++ b/src/qemu/qemu_util.c @@ -0,0 +1,486 @@ +/* + * qemu_util.c: Various util functions for the QEMU driver + * + * Copyright (C) 2013 Red Hat, Inc. Although this file only exists in 2013, it copies from other files with earlier copyrights. Please preserve all of the copyrights from qemu_driver.c (or, for bonus points, take the time to audit which year each individual function was added and later modified while it lived within qemu_driver.c). Same to the qemu_util.h file. + +#include config.h + +#include sys/types.h +#include sys/stat.h +#include unistd.h +#include byteswap.h +#include fcntl.h + +#include qemu_util.h +#include qemu_cgroup.h +#include qemu_migration.h + +#include internal.h + +#include virerror.h +#include domain_conf.h +#include locking/domain_lock.h +#include datatypes.h +#include virlog.h +#include viralloc.h +#include virfile.h Do we need all of these, or can it be trimmed a bit? Should we sort things? +++ b/src/qemu/qemu_util.h @@ -0,0 +1,111 @@ +typedef enum { +QEMU_SAVE_FORMAT_RAW = 0, +QEMU_SAVE_FORMAT_GZIP = 1, +QEMU_SAVE_FORMAT_BZIP2 = 2, +/* + * Deprecated by xz and never used as part of a release + * QEMU_SAVE_FORMAT_LZMA + */ This comment is safe to drop now. We have version control for a reason. ACK with those points addressed. -- 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 1/2] qemu: Split out various utility functions to qemu_util.c
On Thu, Jan 03, 2013 at 05:45:49PM +0100, Peter Krempa wrote: --- po/POTFILES.in | 1 + src/Makefile.am| 1 + src/qemu/qemu_driver.c | 492 ++--- src/qemu/qemu_util.c | 486 src/qemu/qemu_util.h | 111 +++ 5 files changed, 611 insertions(+), 480 deletions(-) create mode 100644 src/qemu/qemu_util.c create mode 100644 src/qemu/qemu_util.h +# include qemu_domain.h + +/* It would be nice to replace 'Qemud' with 'Qemu' but + * this magic string is ABI, so it can't be changed + */ +# define QEMU_SAVE_MAGIC LibvirtQemudSave +# define QEMU_SAVE_PARTIAL LibvirtQemudPart +# define QEMU_SAVE_VERSION 2 + +verify(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL)); + +typedef struct _virQEMUSaveHeader virQEMUSaveHeader; +typedef virQEMUSaveHeader *virQEMUSaveHeaderPtr; +struct _virQEMUSaveHeader { +char magic[sizeof(QEMU_SAVE_MAGIC)-1]; +uint32_t version; +uint32_t xml_len; +uint32_t was_running; +uint32_t compressed; +uint32_t unused[15]; +}; + + +typedef enum { +VIR_DISK_CHAIN_NO_ACCESS, +VIR_DISK_CHAIN_READ_ONLY, +VIR_DISK_CHAIN_READ_WRITE, +} qemuDomainDiskChainMode; + +typedef enum { +QEMU_SAVE_FORMAT_RAW = 0, +QEMU_SAVE_FORMAT_GZIP = 1, +QEMU_SAVE_FORMAT_BZIP2 = 2, +/* + * Deprecated by xz and never used as part of a release + * QEMU_SAVE_FORMAT_LZMA + */ +QEMU_SAVE_FORMAT_XZ = 3, +QEMU_SAVE_FORMAT_LZOP = 4, +/* Note: add new members only at the end. + These values are used in the on-disk format. + Do not change or re-use numbers. */ + +QEMU_SAVE_FORMAT_LAST +} virQEMUSaveFormat; + +VIR_ENUM_DECL(qemuSaveCompression) + + +virDomainObjPtr qemuDomObjFromDomainDriver(virDomainPtr domain, + virQEMUDriverPtr *drv); + +virDomainObjPtr qemuDomObjFromDomain(virDomainPtr domain); + +virDomainObjPtr qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot); + +virDomainSnapshotObjPtr qemuSnapObjFromSnapshot(virDomainObjPtr vm, +virDomainSnapshotPtr snapshot); + +virDomainSnapshotObjPtr qemuSnapObjFromName(virDomainObjPtr vm, +const char *name); All these should go in qemu_domain.{c,h} +int qemuOpenFile(virQEMUDriverPtr driver, + const char *path, + int oflags, + bool *needUnlink, + bool *bypassSecurityDriver); qemu_conf.c +int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCgroupPtr cgroup, + virDomainDiskDefPtr disk, + const char *file, + qemuDomainDiskChainMode mode); qemu_domain.h +int qemuDomainSaveMemory(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path, + const char *domXML, + int compressed, + bool was_running, + unsigned int flags, + enum qemuDomainAsyncJob asyncJob); Not convinced this needs to move at all. +const char *qemuCompressProgramName(int compress); qemu_domain.h In summary, I don't think we should create a qemu_util.{c,h} file - any file named util just ends up as a garbage dumping ground for code that should be better placed elsewhere. See also util/util.h which should be split up. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: Split out snapshot related functions to qemu_snapshot.c
On 01/03/2013 09:45 AM, Peter Krempa wrote: --- po/POTFILES.in |1 + src/Makefile.am |1 + src/qemu/qemu_driver.c | 1699 +--- src/qemu/qemu_snapshot.c | 1752 ++ src/qemu/qemu_snapshot.h | 38 + 5 files changed, 1793 insertions(+), 1698 deletions(-) create mode 100644 src/qemu/qemu_snapshot.c create mode 100644 src/qemu/qemu_snapshot.h Now that I've seen Dan's arguments against 1/2, I tend to agree that qemu_util.c doesn't make much sense. But qemu_snapshot.c still makes sense, so I will still review this one. +++ b/src/qemu/qemu_snapshot.c @@ -0,0 +1,1752 @@ +/* + * qemu_snapshot.c: QEMU snapshot handling + * + * Copyright (C) 2013 Red Hat, Inc. Same comments as in 1/2 about copying over copyright years from the original location. + +#include config.h + +#include sys/types.h +#include sys/stat.h +#include fcntl.h +#include unistd.h + And same comment about sorting includes. +#ifndef __QEMU_SNAPSHOT_H__ +# define __QEMU_SNAPSHOT_H__ + +# include qemu_domain.h + +virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags); + +int qemuDomainRevertToSnapshot(virDomainSnapshotPtr, + unsigned int flags); + +int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, + unsigned int flags); + Interesting subset. Why not move any of these other snapshot-related driver callbacks? qemuDomainSnapshotGetXMLDesc, qemuDomainSnapshotNum, qemuDomainSnapshotListNames, qemuDomainListAllSnapshots, qemuDomainSnapshotNumChildren, qemuDomainSnapshotListChildrenNames, qemuDomainSnapshotListAllChildren, qemuDomainSnapshotLookupByName, qemuDomainSnapshotGetParent, qemuDomainSnapshotCurrent, qemuDomainSnapshotIsCurrent, qemuDomainSnapshotHasMetadata -- 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/2] qemu: Split out snapshot related functions to qemu_snapshot.c
On Thu, Jan 03, 2013 at 10:51:09AM -0700, Eric Blake wrote: On 01/03/2013 09:45 AM, Peter Krempa wrote: --- po/POTFILES.in |1 + src/Makefile.am |1 + src/qemu/qemu_driver.c | 1699 +--- src/qemu/qemu_snapshot.c | 1752 ++ src/qemu/qemu_snapshot.h | 38 + 5 files changed, 1793 insertions(+), 1698 deletions(-) create mode 100644 src/qemu/qemu_snapshot.c create mode 100644 src/qemu/qemu_snapshot.h Now that I've seen Dan's arguments against 1/2, I tend to agree that qemu_util.c doesn't make much sense. But qemu_snapshot.c still makes sense, so I will still review this one. +++ b/src/qemu/qemu_snapshot.c @@ -0,0 +1,1752 @@ +/* + * qemu_snapshot.c: QEMU snapshot handling + * + * Copyright (C) 2013 Red Hat, Inc. Same comments as in 1/2 about copying over copyright years from the original location. + +#include config.h + +#include sys/types.h +#include sys/stat.h +#include fcntl.h +#include unistd.h + And same comment about sorting includes. +#ifndef __QEMU_SNAPSHOT_H__ +# define __QEMU_SNAPSHOT_H__ + +# include qemu_domain.h + +virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags); + +int qemuDomainRevertToSnapshot(virDomainSnapshotPtr, + unsigned int flags); + +int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, + unsigned int flags); + Interesting subset. Why not move any of these other snapshot-related driver callbacks? qemuDomainSnapshotGetXMLDesc, qemuDomainSnapshotNum, qemuDomainSnapshotListNames, qemuDomainListAllSnapshots, qemuDomainSnapshotNumChildren, qemuDomainSnapshotListChildrenNames, qemuDomainSnapshotListAllChildren, qemuDomainSnapshotLookupByName, qemuDomainSnapshotGetParent, qemuDomainSnapshotCurrent, qemuDomainSnapshotIsCurrent, qemuDomainSnapshotHasMetadata IMHO all methods which are directly part of the public API driver struct should be in qemu_driver.c. If we want to split code, then there should be a set of internal helpers which those public API methods call. eg, see how it is done for migration. qemu_driver.c contains qemuDomainMigratePrepare2 which in turn calls qemuMigrationPrepareDirect in qemu_migration.c Basically all the qemu_driver.c code does is convert from the public API types (virDomainPtr) into the private API types (virDomainObjPtr) and then call the main impl code. 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] qemu: fix a segfault in qemuProcessWaitForMonitor
Commit b3f2b4ca5cfe98b08ffdb96f0455e3e333e5ace6 left buf unallocated in the case of QMP capability probing being used, leading to a segfault in strlen in the cleanup path. This patch opens the log and allocates the buffer if QMP probing was used, so we can display the helpful error message. --- src/qemu/qemu_process.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 358757b..2d63cf2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1709,6 +1709,15 @@ cleanup: if (pos != -1 kill(vm-pid, 0) == -1 errno == ESRCH) { /* VM is dead, any other error raised in the interim is probably * not as important as the qemu cmdline output */ +if (qemuCapsUsedQMP(caps)) { +if ((logfd = qemuDomainOpenLog(driver, vm, pos)) 0) +return -1; + +if (VIR_ALLOC_N(buf, buf_size) 0) { +virReportOOMError(); +goto closelog; +} +} qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf)); virReportError(VIR_ERR_INTERNAL_ERROR, _(process exited while connecting to monitor: %s), -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fwrite: silence __wur without using inline
Thanks for fixing that. I'm still puzzled about why the problem happened with libvirt. It's better now that stdio doesn't depend on extern-inline, but I worry that whatever bug the libvirt folks were having with stdio and extern inline might crop up when extern inline is used in other include files. One minor improvement is that we can limit the workaround to glibc versions that have the problem, so I pushed this further change. --- ChangeLog | 6 ++ lib/stdio.in.h | 8 +--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0bae990..714ee4c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2013-01-03 Paul Eggert egg...@cs.ucla.edu + + fwrite: silence __wur only for older glibc versions + * lib/stdio.in.h (fwrite): Limit workaround to glibc 2.4 through 2.15. + This will help us remove this workaround some time in the far future. + 2013-01-03 Eric Blake ebl...@redhat.com fwrite: silence __wur without using inline diff --git a/lib/stdio.in.h b/lib/stdio.in.h index f9765d1..c345499 100644 --- a/lib/stdio.in.h +++ b/lib/stdio.in.h @@ -575,13 +575,15 @@ _GL_CXXALIAS_RPL (fwrite, size_t, _GL_CXXALIAS_SYS (fwrite, size_t, (const void *ptr, size_t s, size_t n, FILE *stream)); -/* Work around glibc bug 11959 +/* Work around bug 11959 when fortifying glibc 2.4 through 2.15 http://sources.redhat.com/bugzilla/show_bug.cgi?id=11959, which sometimes causes an unwanted diagnostic for fwrite calls. This affects only function declaration attributes under certain versions of gcc, and is not needed for C++. */ -# if !defined __cplusplus 0 __USE_FORTIFY_LEVEL \ - 3 (__GNUC__ + (4 = __GNUC_MINOR__)) +# if (0 __USE_FORTIFY_LEVEL \ +__GLIBC__ == 2 4 = __GLIBC_MINOR__ __GLIBC_MINOR__ = 15 \ +3 __GNUC__ + (4 = __GNUC_MINOR__) \ +!defined __cplusplus) # undef fwrite # define fwrite(a, b, c, d) ({size_t __r = fwrite (a, b, c, d); __r; }) # endif -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c
On 01/03/13 18:37, Daniel P. Berrange wrote: On Thu, Jan 03, 2013 at 05:45:49PM +0100, Peter Krempa wrote: --- po/POTFILES.in | 1 + src/Makefile.am| 1 + src/qemu/qemu_driver.c | 492 ++--- src/qemu/qemu_util.c | 486 src/qemu/qemu_util.h | 111 +++ 5 files changed, 611 insertions(+), 480 deletions(-) create mode 100644 src/qemu/qemu_util.c create mode 100644 src/qemu/qemu_util.h ... All these should go in qemu_domain.{c,h} Fair enough. +int qemuOpenFile(virQEMUDriverPtr driver, + const char *path, + int oflags, + bool *needUnlink, + bool *bypassSecurityDriver); qemu_conf.c Hm, qemu_conf.c, okay it's used to open files honoring the config ... +int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virCgroupPtr cgroup, + virDomainDiskDefPtr disk, + const char *file, + qemuDomainDiskChainMode mode); qemu_domain.h +int qemuDomainSaveMemory(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path, + const char *domXML, + int compressed, + bool was_running, + unsigned int flags, + enum qemuDomainAsyncJob asyncJob); Not convinced this needs to move at all. This one needs to be included into qemu_snapshot.c that is created in 2/2 of this series. All functions split out in this patch were chosen to allow that. +const char *qemuCompressProgramName(int compress); qemu_domain.h In summary, I don't think we should create a qemu_util.{c,h} file - any file named util just ends up as a garbage dumping ground for code that should be better placed elsewhere. See also util/util.h which should be split up. Well, as you can see it will ultimately end up somewhere. Unfortunately it this case everything tends to end up in qemu_driver.c. Daniel Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Build failed in Jenkins: libvirt-check #441
See http://honk.sigxcpu.org:8001/job/libvirt-check/441/ -- Started by upstream project libvirt-build build number 482 [workspace] $ /bin/sh -xe /tmp/hudson1110434311877203261.sh + make check Making check in gnulib/lib make[1]: Entering directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib' make check-am make[2]: Entering directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib' make[2]: Nothing to be done for `check-am'. make[2]: Leaving directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib' make[1]: Leaving directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib' Making check in include make[1]: Entering directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include' Making check in libvirt make[2]: Entering directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include/libvirt' make[2]: Nothing to be done for `check'. make[2]: Leaving directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include/libvirt' make[2]: Entering directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include' make[2]: Nothing to be done for `check-am'. make[2]: Leaving directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include' make[1]: Leaving directory `http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include' Making check in src /bin/bash: line 9: 28650 Segmentation fault make $local_target make: *** [check-recursive] Error 1 Build step 'Execute shell' marked build as failure -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix build with optimization enabled
On Thu, Jan 03, 2013 at 10:15:06AM -0700, Eric Blake wrote: Build failure detected by Jenkins autobuilder: http://honk.sigxcpu.org:8001/job/libvirt-build/472/changes * .gnulib: Update to latest, for stdio.h rpl_fwrite fix. That fixed it. Thanks a lot! As of the latest update I'm seeing spurious SIGSEGV on make invocations. I'll diagnose these first until I turn the list mails back on. Cheers, -- Guido --- Pushing under the build-breaker rule. I managed to reproduce the link failure on a RHEL 6 box with CFLAGS='-g -O2', and also that this gnulib update made it go away. * .gnulib 964bbc2...61c7b1e (2): fwrite: silence __wur without using inline update-copyright: avoid copyright notice date corruption .gnulib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gnulib b/.gnulib index 964bbc2..61c7b1e 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 964bbc2d419584e93fe629ddbc40595612f62083 +Subproject commit 61c7b1e32e11e9e40b4d59ab888a807620befcd3 -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Jenkins build is back to normal : libvirt-syntax-check #437
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/437/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/8] Introduce virDomainInterfacesAddresses API
On 01/03/2013 08:46 AM, Michal Privoznik wrote: This API returns dynamically allocated array of IP addresses for all domain interfaces. --- include/libvirt/libvirt.h.in | 49 ++ python/generator.py | 2 ++ src/driver.h | 6 src/libvirt.c| 71 src/libvirt_public.syms | 6 5 files changed, 134 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index cc3fe13..3ee6688 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4563,6 +4563,55 @@ int virDomainFSTrim(virDomainPtr dom, unsigned long long minimum, unsigned int flags); +typedef enum { +VIR_INTERFACE_BROADCAST = (1 0), /* Broadcast address valid. */ +VIR_INTERFACE_PPP = (1 1), /* Interface is point-to-point. */ I think you mean VIR_INTERFACE_PTP, not _PPP. +/* we can add new flags here */ +} virDomainInterfaceType; + +typedef enum { +VIR_IP_ADDR_TYPE_IPV4, +VIR_IP_ADDR_TYPE_IPV6, + +#ifdef VIR_ENUM_SENTINELS +VIR_IP_ADDR_TYPE_LAST +#endif +} virIPAddrType; + + +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress; +typedef virDomainIPAddress *virDomainIPAddressPtr; +struct _virDomainInterfaceIPAddress { +int type; /* virIPAddrType */ +char *addr; /* IP address */ +int prefix; /* IP address prefix */ +char *dstaddr; /* Broadcast address (if @flags VIR_INTERFACE_BROADCAST), + PPP destination address (if @flags VIR_INTERFACE_PPP) */ +}; I dislike creating yet another struct to represent IP address info when we already can do that internally with virSocketAddr, but virSocketAddr is too complicated (and has platform-specific bits to boot) to put it in the public API, and every other IP address in the public API is represented with XML, so I guess this is okay. + +typedef struct _virDomainInterface virDomainInterface; +typedef virDomainInterface *virDomainInterfacePtr; +struct _virDomainInterface { +char *name; /* interface name */ +unsigned int flags; /* virDomainInterfaceType */ +char *hwaddr; /* hardware address */ +unsigned int ip_addrs_count;/* number of items in @ip_addr */ +virDomainIPAddressPtr ip_addrs; /* array of IP addresses */ +}; + +typedef enum { +VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT = 0, /* hypervisor choice */ +VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT = 1, /* use guest agent */ +VIR_DOMAIN_INTERFACE_ADDRS_NWFILTER = 2, /* use nwfilter learning code */ +/* etc ... */ You should remove the etc line. +} virDomainInterfacesAddressesMethod; + + +int virDomainInterfacesAddresses(virDomainPtr domain, + virDomainInterfacePtr **ifaces, + unsigned int method, + unsigned int flags); + /** * virSchedParameterType: * diff --git a/python/generator.py b/python/generator.py index bae4edc..ee39e51 100755 --- a/python/generator.py +++ b/python/generator.py @@ -243,6 +243,7 @@ skipped_types = { 'virEventHandleCallback': No function types in python, 'virEventTimeoutCallback': No function types in python, 'virDomainBlockJobInfoPtr': Not implemented yet, + 'virDomainInterfacePtr': Not implemented yet, } ### @@ -428,6 +429,7 @@ skip_impl = ( 'virNodeGetMemoryParameters', 'virNodeSetMemoryParameters', 'virNodeGetCPUMap', +'virDomainInterfacesAddresses', ) qemu_skip_impl = ( diff --git a/src/driver.h b/src/driver.h index 64d652f..dc93ffb 100644 --- a/src/driver.h +++ b/src/driver.h @@ -914,6 +914,11 @@ typedef int const char *mountPoint, unsigned long long minimum, unsigned int flags); +typedef int +(*virDrvDomainInterfacesAddresses)(virDomainPtr domain, + virDomainInterfacePtr **ifaces, + unsigned int method, + unsigned int flags); /** * _virDriver: @@ -1107,6 +1112,7 @@ struct _virDriver { virDrvNodeGetCPUMap nodeGetCPUMap; virDrvDomainFSTrim domainFSTrim; virDrvDomainSendProcessSignal domainSendProcessSignal; +virDrvDomainInterfacesAddresses domainInterfacesAddresses; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index bf674d1..6f0de36 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20424,3 +20424,74 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainInterfacesAddresses: The repeated
[libvirt] [PATCH 02/10] Check and handle error for virAsprintf() calls. Ignore the return in parallelsMakePoolName() since subsequent check validates name was allocated.
--- src/parallels/parallels_storage.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index e768d88..2908bee 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -146,7 +146,7 @@ static char *parallelsMakePoolName(virConnectPtr conn, const char *path) if (i == 0) name = strdup(path); else -virAsprintf(name, %s-%u, path, i); +ignore_value(virAsprintf(name, %s-%u, path, i)); if (!name) { virReportOOMError(); @@ -310,8 +310,7 @@ static int parallelsAddDiskVolume(virStoragePoolObjPtr pool, if (VIR_ALLOC(def)) goto no_memory; -virAsprintf(def-name, %s-%s, dom-def-name, diskName); -if (!def-name) +if (virAsprintf(def-name, %s-%s, dom-def-name, diskName) 0) goto no_memory; def-type = VIR_STORAGE_VOL_FILE; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/10] Ignore the return status check for vmwareUpdateVMStatus in convenience routine vmwareDomainObjListUpdateDomain
--- src/vmware/vmware_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 3763f40..9c81df8 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -949,7 +949,7 @@ static void vmwareDomainObjListUpdateDomain(void *payload, const void *name ATTR struct vmware_driver *driver = data; virDomainObjPtr vm = payload; virDomainObjLock(vm); -vmwareUpdateVMStatus(driver, vm); +ignore_value(vmwareUpdateVMStatus(driver, vm)); virDomainObjUnlock(vm); } -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/10] Check return on mkdir for LOCKSPACE_DIR
--- tests/virlockspacetest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virlockspacetest.c b/tests/virlockspacetest.c index 8673700..7678396 100644 --- a/tests/virlockspacetest.c +++ b/tests/virlockspacetest.c @@ -293,7 +293,8 @@ static int testLockSpaceResourceLockPath(const void *args ATTRIBUTE_UNUSED) lockspace = virLockSpaceNew(NULL); -mkdir(LOCKSPACE_DIR, 0700); +if (mkdir(LOCKSPACE_DIR, 0700) 0) +goto cleanup; if (virLockSpaceCreateResource(lockspace, LOCKSPACE_DIR /foo) 0) goto cleanup; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/10] Check and handle error for virAsprintf() calls.
--- tests/vmx2xmltest.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 03a8989..2430350 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -179,7 +179,9 @@ testParseVMXFileName(const char *fileName, void *opaque ATTRIBUTE_UNUSED) goto cleanup; } -virAsprintf(src, [%s] %s, datastoreName, directoryAndFileName); +if (virAsprintf(src, [%s] %s, datastoreName, +directoryAndFileName) 0) +goto cleanup; } else if (STRPREFIX(fileName, /)) { /* Found absolute path referencing a file outside a datastore */ src = strdup(fileName); @@ -188,7 +190,8 @@ testParseVMXFileName(const char *fileName, void *opaque ATTRIBUTE_UNUSED) src = NULL; } else { /* Found single file name referencing a file inside a datastore */ -virAsprintf(src, [datastore] directory/%s, fileName); +if (virAsprintf(src, [datastore] directory/%s, fileName) 0) +goto cleanup; } cleanup: -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/10] Resolve CHECKED_RETURN errors found by Coverity
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=880388 This set of patches resolves the CHECKED_RETURN (CWE-252) errors found by Coverity. John Ferlan (10): interface: Check and handle error for virAsprintf() calls. parallels: Check and handle error for virAsprintf() calls. Ignore the return inparallelsMakePoolName() since subsequent check validates name was allocated. rpc: Check status when attempting to set SO_REUSEADDR flag on outgoing connection. On failure, VIR_WARN(), but continue to connect. vmware: Ignore the return status check for vmwareUpdateVMStatus in convenience routine vmwareDomainObjListUpdateDomain xen: Check return status for setting TCP_NODELAY option and generate a VIR_DEBUG message on failure. Allow connection to continue. virlockspacetest: Check return on mkdir for LOCKSPACE_DIR vmx2xmltest: Check and handle error for virAsprintf() calls. xml2vmxtest: Check and handle error for virAsprintf() calls. virsh: Ignore error returns for virBufferTrim(). phyp: Check and handle select() errors from waitsocket(). src/interface/interface_backend_udev.c | 5 ++-- src/parallels/parallels_storage.c | 5 ++-- src/phyp/phyp_driver.c | 42 ++ src/rpc/virnetsocket.c | 4 +++- src/vmware/vmware_driver.c | 2 +- src/xen/xend_internal.c| 6 +++-- tests/virlockspacetest.c | 3 ++- tests/vmx2xmltest.c| 7 -- tests/xml2vmxtest.c| 5 ++-- tools/virsh.c | 4 ++-- 10 files changed, 56 insertions(+), 27 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/10] Ignore error returns for virBufferTrim().
--- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 283194a..9f834e4 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -544,7 +544,7 @@ vshTreePrintInternal(vshControl *ctl, false, indent) 0) goto cleanup; } -virBufferTrim(indent, , -1); +ignore_value(virBufferTrim(indent, , -1)); /* If there was no child device, and we're the last in * a list of devices, then print another blank line */ @@ -552,7 +552,7 @@ vshTreePrintInternal(vshControl *ctl, vshPrint(ctl, %s\n, virBufferCurrentContent(indent)); if (!root) -virBufferTrim(indent, NULL, 2); +ignore_value(virBufferTrim(indent, NULL, 2)); ret = 0; cleanup: return ret; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/10] Check and handle error for virAsprintf() calls.
--- tests/xml2vmxtest.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index 653ab6c..c578109 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -193,8 +193,9 @@ testFormatVMXFileName(const char *src, void *opaque ATTRIBUTE_UNUSED) directoryAndFileName += strspn(directoryAndFileName, ); } -virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName, -directoryAndFileName); +if (virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName, +directoryAndFileName) 0) +goto cleanup; } else if (STRPREFIX(src, /)) { /* Found absolute path */ absolutePath = strdup(src); -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/10] Check return status for setting TCP_NODELAY option and generate a VIR_DEBUG message on failure. Allow connection to continue.
--- src/xen/xend_internal.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 84a25e8..1f779b0 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -91,8 +91,10 @@ do_connect(virConnectPtr xend) /* * try to desactivate slow-start */ -setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start, - sizeof(no_slow_start)); +if (setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start, + sizeof(no_slow_start)) 0) { +VIR_DEBUG(Unable to disable nagle algorithm); +} if (connect(s, (struct sockaddr *)priv-addr, priv-addrlen) == -1) { -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/10] Check and handle select() errors from waitsocket().
--- src/phyp/phyp_driver.c | 42 -- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 8e26b0c..2dabd19 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -76,7 +76,6 @@ static int waitsocket(int socket_fd, LIBSSH2_SESSION * session) { struct timeval timeout; -int rc; fd_set fd; fd_set *writefd = NULL; fd_set *readfd = NULL; @@ -98,9 +97,7 @@ waitsocket(int socket_fd, LIBSSH2_SESSION * session) if (dir LIBSSH2_SESSION_BLOCK_OUTBOUND) writefd = fd; -rc = select(socket_fd + 1, readfd, writefd, NULL, timeout); - -return rc; +return select(socket_fd + 1, readfd, writefd, NULL, timeout); } /* this function is the layer that manipulates the ssh channel itself @@ -131,7 +128,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, while ((channel = libssh2_channel_open_session(session)) == NULL libssh2_session_last_error(session, NULL, NULL, 0) == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } if (channel == NULL) { @@ -140,7 +141,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, while ((rc = libssh2_channel_exec(channel, cmd)) == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } if (rc != 0) { @@ -161,7 +166,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, /* this is due to blocking that would occur otherwise so we loop on * this condition */ if (rc == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } else { break; } @@ -170,7 +179,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, exitcode = 127; while ((rc = libssh2_channel_close(channel)) == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } if (rc == 0) { @@ -735,7 +748,11 @@ phypUUIDTable_Pull(virConnectPtr conn) LIBSSH2_ERROR_EAGAIN) { goto err; } else { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } } } while (!channel); @@ -769,7 +786,12 @@ phypUUIDTable_Pull(virConnectPtr conn) /* this is due to blocking that would occur otherwise * so we loop on this condition */ -waitsocket(sock, session); /* now we wait */ +/* now we wait */ +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} continue; } break; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/10] Check and handle error for virAsprintf() calls.
--- src/interface/interface_backend_udev.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index cc20b98..3231b73 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -654,9 +654,8 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name) ifacedef-data.bridge.stp = stp; /* Members of the bridge */ -virAsprintf(member_path, %s/%s, -udev_device_get_syspath(dev), brif); -if (!member_path) { +if (virAsprintf(member_path, %s/%s, +udev_device_get_syspath(dev), brif) 0) { virReportOOMError(); goto cleanup; } -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/10] Check status when attempting to set SO_REUSEADDR flag on outgoing connection On failure, VIR_WARN(), but continue to connect.
--- src/rpc/virnetsocket.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index ef93892..6684eef 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -470,7 +470,9 @@ int virNetSocketNewConnectTCP(const char *nodename, goto error; } -setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt)); +if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt)) 0) { +VIR_WARN(Unable to enable port reuse); +} if (connect(fd, runp-ai_addr, runp-ai_addrlen) = 0) break; -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/10] Check and handle error for virAsprintf() calls.
On Thu, Jan 03, 2013 at 02:16:13PM -0500, John Ferlan wrote: --- src/interface/interface_backend_udev.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index cc20b98..3231b73 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -654,9 +654,8 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name) ifacedef-data.bridge.stp = stp; /* Members of the bridge */ -virAsprintf(member_path, %s/%s, -udev_device_get_syspath(dev), brif); -if (!member_path) { +if (virAsprintf(member_path, %s/%s, +udev_device_get_syspath(dev), brif) 0) { virReportOOMError(); goto cleanup; } ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/10] Check and handle error for virAsprintf() calls. Ignore the return in parallelsMakePoolName() since subsequent check validates name was allocated.
NB, you should restrict the first line of the commit message to approx 70 characters. Then have a single blank line, followed by the longer description. This ensures that you get sensible subject lines :-) On Thu, Jan 03, 2013 at 02:16:14PM -0500, John Ferlan wrote: --- src/parallels/parallels_storage.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index e768d88..2908bee 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -146,7 +146,7 @@ static char *parallelsMakePoolName(virConnectPtr conn, const char *path) if (i == 0) name = strdup(path); else -virAsprintf(name, %s-%u, path, i); +ignore_value(virAsprintf(name, %s-%u, path, i)); if (!name) { virReportOOMError(); @@ -310,8 +310,7 @@ static int parallelsAddDiskVolume(virStoragePoolObjPtr pool, if (VIR_ALLOC(def)) goto no_memory; -virAsprintf(def-name, %s-%s, dom-def-name, diskName); -if (!def-name) +if (virAsprintf(def-name, %s-%s, dom-def-name, diskName) 0) goto no_memory; def-type = VIR_STORAGE_VOL_FILE; ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/10] Check return status for setting TCP_NODELAY option and generate a VIR_DEBUG message on failure. Allow connection to continue.
On Thu, Jan 03, 2013 at 02:16:17PM -0500, John Ferlan wrote: --- src/xen/xend_internal.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 84a25e8..1f779b0 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -91,8 +91,10 @@ do_connect(virConnectPtr xend) /* * try to desactivate slow-start */ -setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start, - sizeof(no_slow_start)); +if (setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start, + sizeof(no_slow_start)) 0) { +VIR_DEBUG(Unable to disable nagle algorithm); +} Hmm, I'd just make this ignore_value() i think - it is something we expect to fail with UNIX sockets Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] Check return on mkdir for LOCKSPACE_DIR
On Thu, Jan 03, 2013 at 02:16:18PM -0500, John Ferlan wrote: --- tests/virlockspacetest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virlockspacetest.c b/tests/virlockspacetest.c index 8673700..7678396 100644 --- a/tests/virlockspacetest.c +++ b/tests/virlockspacetest.c @@ -293,7 +293,8 @@ static int testLockSpaceResourceLockPath(const void *args ATTRIBUTE_UNUSED) lockspace = virLockSpaceNew(NULL); -mkdir(LOCKSPACE_DIR, 0700); +if (mkdir(LOCKSPACE_DIR, 0700) 0) +goto cleanup; if (virLockSpaceCreateResource(lockspace, LOCKSPACE_DIR /foo) 0) goto cleanup; ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/10] Check and handle error for virAsprintf() calls.
On Thu, Jan 03, 2013 at 02:16:19PM -0500, John Ferlan wrote: --- tests/vmx2xmltest.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 03a8989..2430350 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -179,7 +179,9 @@ testParseVMXFileName(const char *fileName, void *opaque ATTRIBUTE_UNUSED) goto cleanup; } -virAsprintf(src, [%s] %s, datastoreName, directoryAndFileName); +if (virAsprintf(src, [%s] %s, datastoreName, +directoryAndFileName) 0) +goto cleanup; } else if (STRPREFIX(fileName, /)) { /* Found absolute path referencing a file outside a datastore */ src = strdup(fileName); @@ -188,7 +190,8 @@ testParseVMXFileName(const char *fileName, void *opaque ATTRIBUTE_UNUSED) src = NULL; } else { /* Found single file name referencing a file inside a datastore */ -virAsprintf(src, [datastore] directory/%s, fileName); +if (virAsprintf(src, [datastore] directory/%s, fileName) 0) +goto cleanup; } cleanup: ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/10] Check and handle error for virAsprintf() calls.
On Thu, Jan 03, 2013 at 02:16:20PM -0500, John Ferlan wrote: --- tests/xml2vmxtest.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index 653ab6c..c578109 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -193,8 +193,9 @@ testFormatVMXFileName(const char *src, void *opaque ATTRIBUTE_UNUSED) directoryAndFileName += strspn(directoryAndFileName, ); } -virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName, -directoryAndFileName); +if (virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName, +directoryAndFileName) 0) +goto cleanup; } else if (STRPREFIX(src, /)) { /* Found absolute path */ absolutePath = strdup(src); ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/10] Check status when attempting to set SO_REUSEADDR flag on outgoing connection On failure, VIR_WARN(), but continue to connect.
On Thu, Jan 03, 2013 at 02:16:15PM -0500, John Ferlan wrote: --- src/rpc/virnetsocket.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index ef93892..6684eef 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -470,7 +470,9 @@ int virNetSocketNewConnectTCP(const char *nodename, goto error; } -setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt)); +if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt)) 0) { +VIR_WARN(Unable to enable port reuse); +} if (connect(fd, runp-ai_addr, runp-ai_addrlen) = 0) break; Hmm, not sure I agree with this. If this is something that should not occurr, then we should virReportError. If it is something we expect to occur, then VIR_WARN will annoy people with irrelevant messages. My inclination is to treat it as a fatal error Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] Ignore error returns for virBufferTrim().
On Thu, Jan 03, 2013 at 02:16:21PM -0500, John Ferlan wrote: --- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 283194a..9f834e4 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -544,7 +544,7 @@ vshTreePrintInternal(vshControl *ctl, false, indent) 0) goto cleanup; } -virBufferTrim(indent, , -1); +ignore_value(virBufferTrim(indent, , -1)); /* If there was no child device, and we're the last in * a list of devices, then print another blank line */ @@ -552,7 +552,7 @@ vshTreePrintInternal(vshControl *ctl, vshPrint(ctl, %s\n, virBufferCurrentContent(indent)); if (!root) -virBufferTrim(indent, NULL, 2); +ignore_value(virBufferTrim(indent, NULL, 2)); ret = 0; cleanup: return ret; I must say I don't see the point in the return value for virBufferTrim. I think I'd recommend turning it into a 'void' function Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] Check and handle select() errors from waitsocket().
On Thu, Jan 03, 2013 at 02:16:22PM -0500, John Ferlan wrote: --- src/phyp/phyp_driver.c | 42 -- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 8e26b0c..2dabd19 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -76,7 +76,6 @@ static int waitsocket(int socket_fd, LIBSSH2_SESSION * session) { struct timeval timeout; -int rc; fd_set fd; fd_set *writefd = NULL; fd_set *readfd = NULL; @@ -98,9 +97,7 @@ waitsocket(int socket_fd, LIBSSH2_SESSION * session) if (dir LIBSSH2_SESSION_BLOCK_OUTBOUND) writefd = fd; -rc = select(socket_fd + 1, readfd, writefd, NULL, timeout); - -return rc; +return select(socket_fd + 1, readfd, writefd, NULL, timeout); } /* this function is the layer that manipulates the ssh channel itself @@ -131,7 +128,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, while ((channel = libssh2_channel_open_session(session)) == NULL libssh2_session_last_error(session, NULL, NULL, 0) == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } if (channel == NULL) { @@ -140,7 +141,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, while ((rc = libssh2_channel_exec(channel, cmd)) == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } if (rc != 0) { @@ -161,7 +166,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, /* this is due to blocking that would occur otherwise so we loop on * this condition */ if (rc == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } else { break; } @@ -170,7 +179,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int *exit_status, exitcode = 127; while ((rc = libssh2_channel_close(channel)) == LIBSSH2_ERROR_EAGAIN) { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } if (rc == 0) { @@ -735,7 +748,11 @@ phypUUIDTable_Pull(virConnectPtr conn) LIBSSH2_ERROR_EAGAIN) { goto err; } else { -waitsocket(sock, session); +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} } } } while (!channel); @@ -769,7 +786,12 @@ phypUUIDTable_Pull(virConnectPtr conn) /* this is due to blocking that would occur otherwise * so we loop on this condition */ -waitsocket(sock, session); /* now we wait */ +/* now we wait */ +if (waitsocket(sock, session) 0 errno != EINTR) { +virReportSystemError(errno, %s, + _(unable to wait on libssh2 socket)); +goto err; +} continue; } break; ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Resolve CHECKED_RETURN errors found by Coverity
On Thu, Jan 03, 2013 at 02:16:12PM -0500, John Ferlan wrote: Bug: https://bugzilla.redhat.com/show_bug.cgi?id=880388 This set of patches resolves the CHECKED_RETURN (CWE-252) errors found by Coverity. John Ferlan (10): interface: Check and handle error for virAsprintf() calls. parallels: Check and handle error for virAsprintf() calls. Ignore the return inparallelsMakePoolName() since subsequent check validates name was allocated. rpc: Check status when attempting to set SO_REUSEADDR flag on outgoing connection. On failure, VIR_WARN(), but continue to connect. vmware: Ignore the return status check for vmwareUpdateVMStatus in convenience routine vmwareDomainObjListUpdateDomain xen: Check return status for setting TCP_NODELAY option and generate a VIR_DEBUG message on failure. Allow connection to continue. virlockspacetest: Check return on mkdir for LOCKSPACE_DIR vmx2xmltest: Check and handle error for virAsprintf() calls. xml2vmxtest: Check and handle error for virAsprintf() calls. virsh: Ignore error returns for virBufferTrim(). phyp: Check and handle select() errors from waitsocket(). There are a number of issues with vifAsprintf(). As a further patch I think you should add ATTRIBUTE_RETURN_CHECK to this function, so we see the problems immediately rather than relying on coverity. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/10] Check status when attempting to set SO_REUSEADDR flag on outgoing connection On failure, VIR_WARN(), but continue to connect.
On 01/03/2013 02:34 PM, Daniel P. Berrange wrote: On Thu, Jan 03, 2013 at 02:16:15PM -0500, John Ferlan wrote: --- src/rpc/virnetsocket.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index ef93892..6684eef 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -470,7 +470,9 @@ int virNetSocketNewConnectTCP(const char *nodename, goto error; } -setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt)); +if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt)) 0) { +VIR_WARN(Unable to enable port reuse); +} if (connect(fd, runp-ai_addr, runp-ai_addrlen) = 0) break; Hmm, not sure I agree with this. If this is something that should not occurr, then we should virReportError. If it is something we expect to occur, then VIR_WARN will annoy people with irrelevant messages. I asked about this yesterday and Michal P responded. The REUSEADDR is a more of a hint for connections, see the end of: https://www.redhat.com/archives/libvir-list/2013-January/msg00064.html I don't mind either way. My inclination is to treat it as a fatal error Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemu: Add a hash table for the shared disks
On 01/02/2013 07:37 AM, Osier Yang wrote: This introduces a hash table for qemu driver, to store the shared disk's info as (@major:minor, @ref_count). @ref_count is the number of domains which shares the disk. Since we only care about if the disk support unprivileged SG_IO commands, and the SG_IO commands only make sense for block disk, this patch only manages (add/remove hash entry) the shared disk for block disk. * src/qemu/qemu_conf.h: (Add member 'sharedDisks' of type virHashTablePtr; Declare helpers qemuGetSharedDiskKey, qemuAddSharedDisk and qemuRemoveSharedDisk) * src/qemu/qemu_conf.c (Implement the 3 helpers) * src/qemu/qemu_process.c (Update 'sharedDisks' when domain starting and shutdown) * src/qemu/qemu_driver.c (Update 'sharedDisks' when attaching or detaching disk). --- src/qemu/qemu_conf.c| 86 +++ src/qemu/qemu_conf.h| 12 ++ src/qemu/qemu_driver.c | 22 src/qemu/qemu_process.c | 15 4 files changed, 135 insertions(+), 0 deletions(-) ACK, as I'd like to see this go in sooner rather than later, and what you have works. However... diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c6deb10..8440247 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -552,3 +552,89 @@ qemuDriverCloseCallbackRunAll(virQEMUDriverPtr driver, virHashForEach(driver-closeCallbacks, qemuDriverCloseCallbackRun, data); } + +/* Construct the hash key for sharedDisks as major:minor */ +char * +qemuGetSharedDiskKey(const char *disk_path) +{ Why are we converting major and minor into a malloc'd char*,... +int major, minor; Here, I'd use 'maj' and 'min' to avoid any shadowing of the major() and minor() macros. +int +qemuAddSharedDisk(virHashTablePtr sharedDisks, + const char *disk_path) +{ +size_t *ref = NULL; +char *key = NULL; + +if (!(key = qemuGetSharedDiskKey(disk_path))) +return -1; + +if ((ref = virHashLookup(sharedDisks, key))) { ...when you could just use a single int value comprising the combination of major and minor as the hash key, if only you had used virHashCreateFull() with custom comparators. That would be more efficient (no need to malloc each kay, comparisons are much faster as an integer compare instead of a strcmp, and so on). It may be worth a followup patch that re-does the hash table to be more efficient. +++ b/src/qemu/qemu_driver.c @@ -843,6 +843,9 @@ qemuStartup(bool privileged, if ((qemu_driver-inactivePciHostdevs = pciDeviceListNew()) == NULL) goto error; +if (!(qemu_driver-sharedDisks = virHashCreate(30, NULL))) Here's where the virHashCreateFull would let you avoid having to convert the key into a string. -- 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: fix a segfault in qemuProcessWaitForMonitor
On 01/03/2013 11:15 AM, Ján Tomko wrote: Commit b3f2b4ca5cfe98b08ffdb96f0455e3e333e5ace6 left buf unallocated in the case of QMP capability probing being used, leading to a segfault in strlen in the cleanup path. This patch opens the log and allocates the buffer if QMP probing was used, so we can display the helpful error message. --- src/qemu/qemu_process.c |9 + 1 files changed, 9 insertions(+), 0 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
Re: [libvirt] [PATCH] fwrite: silence __wur without using inline
On 01/03/2013 11:16 AM, Paul Eggert wrote: Thanks for fixing that. I'm still puzzled about why the problem happened with libvirt. Why libvirt, and not detected elsewhere? Probably because libvirt does this at configure time: AH_VERBATIM([FORTIFY_SOURCE], [/* Enable compile-time and run-time bounds-checking, and some warnings, without upsetting newer glibc. */ #if !defined _FORTIFY_SOURCE defined __OPTIMIZE__ __OPTIMIZE__ # define _FORTIFY_SOURCE 2 #endif and the inline in question is only turned on when _FORTIFY_SOURCE is enabled. Packages where no one uses _FORTIFY_SOURCE by default would never hit the issue. It's better now that stdio doesn't depend on extern-inline, but I worry that whatever bug the libvirt folks were having with stdio and extern inline might crop up when extern inline is used in other include files. The elided code was not using _GL_EXTERN_INLINE, but _GL_INLINE. They have different semantics, but I'm hard-pressed to say _which_ semantics are right from just those names. If I understand correctly, we _want_ the semantics where the inline function is _always_ inlined and never emitted as a linkable entry point, but I'd have to refer back to C99 and the gcc manual to see which use of inline and possible __attribute__ combinations guarantees that point. I've seen code that uses the name ELIDABLE_INLINE for the specific combination we want used in header files; maybe it's worth adding a name _GL_ELIDABLE_INLINE into m4/extern-inline.m4 to make it easier to use the results. One minor improvement is that we can limit the workaround to glibc versions that have the problem, so I pushed this further change. Thanks for that improvement. -- 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] fwrite: silence __wur without using inline
On 01/03/13 12:23, Eric Blake wrote: The elided code was not using _GL_EXTERN_INLINE, but _GL_INLINE. They have different semantics, but I'm hard-pressed to say _which_ semantics are right from just those names. Standard C semantics. _GL_EXTERN_INLINE is C99/C11 extern inline. _GL_INILNE is C99/C11 inline. If I understand correctly, we _want_ the semantics where the inline function is _always_ inlined and never emitted as a linkable entry point, but I'd have to refer back to C99 and the gcc manual to see which use of inline and possible __attribute__ combinations guarantees that point. Can't be done in Standard C, as far as I know. With GNU C it can be done with __attribute__((__always_inline__)). Why is it important that it not be a linkable entry point? maybe it's worth adding a name _GL_ELIDABLE_INLINE into m4/extern-inline.m4 to make it easier to use the results. Sorry, I guess I don't see how this would work. That is, I can see how _GL_ELIDABLE_INLINE would expand to 'inline __attribute__((__always_inline__))' in GNU C, but what would it expand to in Standard C? It can't be expanded to nothing, since you don't want a linkable entry point, and it can't be expanded to 'static', at least not in a header, because the resulting function couldn't be called from an extern inline function. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fwrite: silence __wur without using inline
On 01/03/2013 01:48 PM, Paul Eggert wrote: Can't be done in Standard C, as far as I know. Oh well, not worth it then. With GNU C it can be done with __attribute__((__always_inline__)). Why is it important that it not be a linkable entry point? At least in the case libvirt was hitting, multiple files used fwrite, which in turn meant multiple linkable entries for rpl_fwrite were emitted when linking things together; but because they weren't marked 'static', the linker didn't like us. If we are going to have a wrapper inline function in a header, then we want to ensure that the wrapper does not cause duplicate links. maybe it's worth adding a name _GL_ELIDABLE_INLINE into m4/extern-inline.m4 to make it easier to use the results. Sorry, I guess I don't see how this would work. That is, I can see how _GL_ELIDABLE_INLINE would expand to 'inline __attribute__((__always_inline__))' in GNU C, but what would it expand to in Standard C? It can't be expanded to nothing, since you don't want a linkable entry point, and it can't be expanded to 'static', at least not in a header, because the resulting function couldn't be called from an extern inline function. Indeed, that's an annoying limitation. Which goes to show that what we did for fwrite (avoid inline altogether, and instead use GNU C instead of Standard C, to get the workaround we wanted) is really all the best we can do - we have to use a case-by-case analysis of WHY a wrapper is in the header, and either pull the wrapper out of a header and into a real function, or rely on compiler extensions for the cases where the wrapper only makes sense for that compiler. -- 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 3/6] docs: Add docs and rng schema for new XML tag sgio
On 01/02/2013 07:37 AM, Osier Yang wrote: This introduces new XML tag sgio for disk, its valid values are filtered and unfiltered, setting it as filtered will set the disk's unpriv_sgio to 0, and unfiltered to set it as 1, which allows the unprivileged SG_IO commands. --- docs/formatdomain.html.in | 14 ++- docs/schemas/domaincommon.rng | 54 +++- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94df6f8..5e37b92 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1395,7 +1395,19 @@ rawio='yes', rawio capability will be enabled for all disks in the domain (because, in the case of QEMU, this capability can only be set on a per-process basis). This attribute is only -valid when device is lun. +valid when device is lun. NB, coderawio/code intends to +confine the capability per-device, however, current QEMU +implementation gives the domain process broader capability +than that (per-process basis, affects all the domain disks). +To confine the capability as much as possible for QEMU driver +as this stage, codesgio/code is recommended, it's more +secure than coderawio/code. +The optional codesgio/code attribute indicates whether the For consistency with how we did it for 'rawio': The optional codesgio/code attribute (span class=sincesince 1.0.2/span) indicates... +kernel will filter unprivileged SG_IO commands for the disk, +valid settings are filtered or unfiltered. Defaults to +filtered. Same with coderawio/code, codesgio/code s/Same with/Similar to/ +is only valid for device 'lun'. +span class=sincesince 1.0.2/span ...then drop the span here. +group + attribute name=device +choice + valuelun/value +/choice Technically, the choice isn't needed here (but it doesn't hurt either). ACK with the grammar cleaned up. -- 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 02/10] Check and handle error for virAsprintf() calls. Ignore the return in parallelsMakePoolName() since subsequent check validates name was allocated.
On 01/03/2013 12:32 PM, Daniel P. Berrange wrote: NB, you should restrict the first line of the commit message to approx 70 characters. Then have a single blank line, followed by the longer description. This ensures that you get sensible subject lines :-) Also, using the same subject line across multiple patches can cause grief to backporting efforts. Inserting a prefix to say which portion of code is touched helps to both remove the ambiguity and make it easier to see at a glance why there are multiple patches. I've modified the commit message to: parallels: check and handle error for virAsprintf() calls Ignore the return in parallelsMakePoolName() since subsequent check validates name was allocated. ACK and I've now pushed 1 and 2, and will continue reading through the series. Congratulations on your first libvirt 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
[libvirt] libvirt unable to start domains
Hi, I updated my libvirt git clone and install yesterday (after about a month or so) and now I can't start domains: $ virsh start fedora18 error: Failed to start domain fedora18 error: unsupported configuration: This QEMU does not support QXL graphics adapters While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila Fedora 18 repo. Just to be clear, qemu was not updated before the problem appeared, only libvirt. Here is the libvirtd log: http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB). Also attaching the domain config. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 domain type='kvm' namefedora18/name uuidd64e1338-da51-c2df-578d-12db0641cde6/uuid titleFedora 18/title metadata boxes:gnome-boxes xmlns:boxes=http://live.gnome.org/Boxes/; os-stateinstalled/os-state os-idhttp://fedoraproject.org/fedora/18/os-id media-idhttp://fedoraproject.org/fedora/18:1/media-id /boxes:gnome-boxes /metadata memory unit='KiB'1179648/memory currentMemory unit='KiB'1179648/currentMemory vcpu placement='static'4/vcpu os type arch='x86_64' machine='pc-1.2'hvm/type boot dev='hd'/ /os features acpi/ apic/ /features cpu mode='host-model' model fallback='allow'/ topology sockets='1' cores='2' threads='2'/ /cpu clock offset='utc' timer name='rtc' tickpolicy='catchup'/ timer name='pit' tickpolicy='delay'/ /clock on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashdestroy/on_crash pm suspend-to-mem enabled='no'/ suspend-to-disk enabled='no'/ /pm devices emulator/usr/bin/qemu-kvm/emulator disk type='file' device='disk' driver name='qemu' type='qcow2' cache='none'/ source file='/home/zeenix/.local/share/gnome-boxes/images/fedora18'/ target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ /disk disk type='file' device='cdrom' driver name='qemu' type='raw'/ target dev='hdc' bus='ide'/ readonly/ address type='drive' controller='0' bus='1' target='0' unit='0'/ /disk controller type='usb' index='0' address type='pci' domain='0x' bus='0x00' slot='0x01' function='0x2'/ /controller controller type='ide' index='0' address type='pci' domain='0x' bus='0x00' slot='0x01' function='0x1'/ /controller controller type='virtio-serial' index='0' address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/ /controller interface type='user' mac address='52:54:00:93:de:4b'/ address type='pci' domain='0x' bus='0x00' slot='0x03' function='0x0'/ /interface serial type='pty' target port='0'/ /serial console type='pty' target type='serial' port='0'/ /console channel type='spicevmc' target type='virtio' name='com.redhat.spice.0'/ address type='virtio-serial' controller='0' bus='0' port='1'/ /channel input type='tablet' bus='usb'/ input type='mouse' bus='ps2'/ graphics type='spice' autoport='yes'/ sound model='ac97' address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/ /sound video model type='qxl' vram='65536' heads='1'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /video memballoon model='virtio' address type='pci' domain='0x' bus='0x00' slot='0x08' function='0x0'/ /memballoon /devices /domain -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Install virtlockd.{socket, service} non executable
since they're not scripts but systemd service files. --- src/Makefile.am |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index f7a9b91..0cfc1ed 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1687,9 +1687,9 @@ DISTCLEANFILES += virtlockd.service virtlockd.socket install-systemd: virtlockd.service virtlockd.socket install-sysconfig $(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR) - $(INSTALL_SCRIPT) virtlockd.service \ + $(INSTALL_DATA) virtlockd.service \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/ - $(INSTALL_SCRIPT) virtlockd.socket \ + $(INSTALL_DATA) virtlockd.socket \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/ uninstall-systemd: uninstall-sysconfig -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Install virtlockd.{socket, service} non executable
On 01/03/2013 03:05 PM, Guido Günther wrote: since they're not scripts but systemd service files. --- src/Makefile.am |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK. diff --git a/src/Makefile.am b/src/Makefile.am index f7a9b91..0cfc1ed 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1687,9 +1687,9 @@ DISTCLEANFILES += virtlockd.service virtlockd.socket install-systemd: virtlockd.service virtlockd.socket install-sysconfig $(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR) - $(INSTALL_SCRIPT) virtlockd.service \ + $(INSTALL_DATA) virtlockd.service \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/ - $(INSTALL_SCRIPT) virtlockd.socket \ + $(INSTALL_DATA) virtlockd.socket \ $(DESTDIR)$(SYSTEMD_UNIT_DIR)/ uninstall-systemd: uninstall-sysconfig -- 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] The -vga CLI arg accepts qxl in qemu 1.2+ always
The -vga command always accepts qxl in 1.2 and newer. --- src/qemu/qemu_capabilities.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f49a31c..c3ab488 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2282,6 +2282,7 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps) qemuCapsSet(caps, QEMU_CAPS_NO_USER_CONFIG); qemuCapsSet(caps, QEMU_CAPS_NETDEV_BRIDGE); qemuCapsSet(caps, QEMU_CAPS_SECCOMP_SANDBOX); +qemuCapsSet(caps, QEMU_CAPS_VGA_QXL); } -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt unable to start domains
On Thu, Jan 3, 2013 at 3:44 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: Hi, I updated my libvirt git clone and install yesterday (after about a month or so) and now I can't start domains: $ virsh start fedora18 error: Failed to start domain fedora18 error: unsupported configuration: This QEMU does not support QXL graphics adapters While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila Fedora 18 repo. Just to be clear, qemu was not updated before the problem appeared, only libvirt. Here is the libvirtd log: http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB). Also attaching the domain config. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 Try the patch I sent in response to your original message. Not sure if that's the 100% fix we want. Need to check if we should also be checking for SPICE support in qemu_command.c but it should at the very least get you going for now. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt unable to start domains
On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: Hi, I updated my libvirt git clone and install yesterday (after about a month or so) and now I can't start domains: $ virsh start fedora18 error: Failed to start domain fedora18 error: unsupported configuration: This QEMU does not support QXL graphics adapters While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila Fedora 18 repo. Just to be clear, qemu was not updated before the problem appeared, only libvirt. Here is the libvirtd log: http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB). Also attaching the domain config. If my `git bisect run` script worked, I've found the commit introducing this issue: 4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit commit 4c993d8ab5473334e9d4155faa913476ada39069 Author: Guannan Ren g...@redhat.com Date: Fri Dec 14 15:06:31 2012 +0800 qemu: add qemu vga devices caps and one cap to mark them usable QEMU_CAPS_DEVICE_QXL -device qxl QEMU_CAPS_DEVICE_VGA -device VGA QEMU_CAPS_DEVICE_CIRRUS_VGA -device cirrus-vga QEMU_CAPS_DEVICE_VMWARE_SVGA -device vmware-svga QEMU_CAPS_DEVICE_VIDEO_PRIMARY /* safe to use -device XXX for primary video device */ Fix a typo in qemuCapsObjectTypes, the string 'qxl' here should be -device qxl rather than -vga [...|qxl|..] :04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758 5afc50956ad09cf6e740ce75800d746a53513271 M src :04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M tests bisect run success FWIW, problem is not reproducible against the parent commit 34ca5684970fc5e4be0ec29809b73dea7be2d84f. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fwrite: silence __wur without using inline
On 01/03/2013 01:00 PM, Eric Blake wrote: in the case libvirt was hitting, multiple files used fwrite, which in turn meant multiple linkable entries for rpl_fwrite were emitted when linking things together; but because they weren't marked 'static', the linker didn't like us. OK, but surely this problem would happen with any extern symbol that gnulib defines. I.e., it's not a problem specific to extern inline functions; it's a problem with externs in general. Isn't the right way to handle this, from the gnulib point of view, the lib-symbol-visibility module? Shouldn't libvirt be using that? If libvirt was compiled with -fvisibility=hidden then any miscellanous extern functions that it defined would be local to the libvirt .so, and wouldn't clash with similarly-named functions in other shared objects. what we did for fwrite (avoid inline altogether, and instead use GNU C instead of Standard C, to get the workaround we wanted) is really all the best we can do I hope that's not the case, because it pretty much means that there's no portable way to use inline functions in gnulib headers that contribute to shared objects. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt unable to start domains
On Fri, Jan 4, 2013 at 1:44 AM, Doug Goldstein car...@gentoo.org wrote: On Thu, Jan 3, 2013 at 3:44 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: Hi, I updated my libvirt git clone and install yesterday (after about a month or so) and now I can't start domains: $ virsh start fedora18 error: Failed to start domain fedora18 error: unsupported configuration: This QEMU does not support QXL graphics adapters While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila Fedora 18 repo. Just to be clear, qemu was not updated before the problem appeared, only libvirt. Here is the libvirtd log: http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB). Also attaching the domain config. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 Try the patch I sent in response to your original message. Not sure if that's the 100% fix we want. Need to check if we should also be checking for SPICE support in qemu_command.c but it should at the very least get you going for now. Thanks, I'll try that. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] conf: Parse and format the new XML
On 01/02/2013 07:37 AM, Osier Yang wrote: Like rawio, sgio is only allowed for block disk of device type lun. It doesn't default disk-sgio to filtered when parsing, as it won't be able to distinguish explicitly requested filtered and a default filtered in driver then. We have to error out for explicit request when the kernel doesn't support the new sysfs knob unpriv_sgio, however, for defaulted filtered, we can just ignore it if the kernel doesn't support unpriv_sgio. --- src/conf/domain_conf.c | 55 +++- src/conf/domain_conf.h | 10 ...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml | 32 +++ tests/qemuxml2xmltest.c|1 + 4 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml +enum virDomainDiskSGIO { +VIR_DOMAIN_DISK_SGIO_DEFAULT = 0, +VIR_DOMAIN_DISK_SGIO_FILTERED, +VIR_DOMAIN_DISK_SGIO_UNFILTERED, + +VIR_DOMAIN_DISK_SGIO_LAST +}; + typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; struct _virDomainBlockIoTuneInfo { unsigned long long total_bytes_sec; @@ -638,6 +646,7 @@ struct _virDomainDiskDef { virStorageEncryptionPtr encryption; bool rawio_specified; int rawio; /* no = 0, yes = 1 */ Don't know why we didn't make this 'bool', but that's pre-existing and would be a separate cleanup patch. +int sgio; I'd add /* enum virDomainDiskSGIO */, to make it easier to see what goes in this int. 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] libvirt unable to start domains
On Thu, Jan 3, 2013 at 5:58 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: Hi, I updated my libvirt git clone and install yesterday (after about a month or so) and now I can't start domains: $ virsh start fedora18 error: Failed to start domain fedora18 error: unsupported configuration: This QEMU does not support QXL graphics adapters While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila Fedora 18 repo. Just to be clear, qemu was not updated before the problem appeared, only libvirt. Here is the libvirtd log: http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB). Also attaching the domain config. If my `git bisect run` script worked, I've found the commit introducing this issue: 4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit commit 4c993d8ab5473334e9d4155faa913476ada39069 Author: Guannan Ren g...@redhat.com Date: Fri Dec 14 15:06:31 2012 +0800 qemu: add qemu vga devices caps and one cap to mark them usable QEMU_CAPS_DEVICE_QXL -device qxl QEMU_CAPS_DEVICE_VGA -device VGA QEMU_CAPS_DEVICE_CIRRUS_VGA -device cirrus-vga QEMU_CAPS_DEVICE_VMWARE_SVGA -device vmware-svga QEMU_CAPS_DEVICE_VIDEO_PRIMARY /* safe to use -device XXX for primary video device */ Fix a typo in qemuCapsObjectTypes, the string 'qxl' here should be -device qxl rather than -vga [...|qxl|..] :04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758 5afc50956ad09cf6e740ce75800d746a53513271 M src :04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M tests bisect run success FWIW, problem is not reproducible against the parent commit 34ca5684970fc5e4be0ec29809b73dea7be2d84f. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 Basically for some reason we tried to build the command line up as qemu-kvm ... -vga qxl instead of -device qxl-vga which is what I thought Guannan was aiming for. I could be wrong though. I'll take another look at it tonight. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] The -vga CLI arg accepts qxl in qemu 1.2+ always
On 01/03/2013 04:42 PM, Doug Goldstein wrote: The -vga command always accepts qxl in 1.2 and newer. Are you sure? I thought qemu ./configure allows a choice of which graphics engines to support, and that you can compile qxl out (distros don't do that, but a self-built qemu might support -vga but not QXL). I think that rather than blindly setting this cap, we need to find the right QMP command to check which graphics engines are supported. --- src/qemu/qemu_capabilities.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f49a31c..c3ab488 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2282,6 +2282,7 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps) qemuCapsSet(caps, QEMU_CAPS_NO_USER_CONFIG); qemuCapsSet(caps, QEMU_CAPS_NETDEV_BRIDGE); qemuCapsSet(caps, QEMU_CAPS_SECCOMP_SANDBOX); +qemuCapsSet(caps, QEMU_CAPS_VGA_QXL); } -- 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] RFC: An embedded mode for QEMU/LXC drivers
On 01/02/2013 09:55 PM, Richard W.M. Jones wrote: On Wed, Jan 02, 2013 at 03:36:54PM +, Daniel P. Berrange wrote: This is something I was thinking about a little over the christmas break. I've no intention of implementing this in the immediate future, but wanted to post it while it was fresh in my mind. Historically we have had 2 ways of using the stateful drivers like QEMU/LXC/UML/etc. - system mode - privileged libvirtd, one per host, started at boot - session mode - unprivileged libvirtd, one per non-root user, autostarted This leads me to wonder whether it is worth exploring the idea of a new type of libvirt connection. - embed mode - no libvirtd, driver runs in application context Seems like an excellent idea. Seconded. But I also have to wonder if Dan's work-in-progress on fine-grain ACLs could also be used for the case of isolating domains under the control of libguestfs so that virt-manager/oVirt/what-not can't control the libguestfs domains, even though all the domains are managed by the same libvirtd. In other words, you may be able to achieve embedded semantics by means of ACLs. -- 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] problem in pciFindStubDriver
For Xen PV guests, pciback is the only working driver, pci-stub doesn't work. Current function finds pci-stub driver first, if pci-stub doesn't exist, find pciback. It won't work for Xen PV guests since it will find pci-stub driver and return, but in fact it needs pciback. One way is to prefer pciback rather than pci-stub like in following patch. But that will change the behaviour of other drivers too. Is there any preferred aprroach to handle this? diff --git a/src/util/virpci.c b/src/util/virpci.c index 5971764..c0a1c05 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -861,22 +861,22 @@ pciFindStubDriver(void) int probed = 0; recheck: -if (pciDriverDir(drvpath, pci-stub) 0) { +if (pciDriverDir(drvpath, pciback) 0) { return NULL; } if (virFileExists(drvpath)) { VIR_FREE(drvpath); -return pci-stub; +return pciback; } -if (pciDriverDir(drvpath, pciback) 0) { +if (pciDriverDir(drvpath, pci-stub) 0) { return NULL; } if (virFileExists(drvpath)) { VIR_FREE(drvpath); -return pciback; +return pci-stub; } VIR_FREE(drvpath); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu
在 2013-01-03四的 16:13 +0100,Ján Tomko写道: On 12/26/12 02:00, liguang wrote: @@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, * When QEMU grows support for 1 PCI domain, then pci.0 change * to pciNN.0 where NN is the domain number */ -if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) +if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) { +virBufferAsprintf(buf, ,bus=pci-bridge%d, info-addr.pci.bus); +} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) { virBufferAsprintf(buf, ,bus=pci.0); Is there any way (or plan) to use more pci buses with QEMU other than with the pci bridges? If not, we could just name the bridges pci.%d. (If we index the bridges from 1). as far as I know, qemu can't use multi-pci-bus, so only pci.0 accepted now. -else +} else { virBufferAsprintf(buf, ,bus=pci); +} if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) virBufferAddLit(buf, ,multifunction=on); else if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF) @@ -3455,6 +3460,32 @@ error: return NULL; } +char * +qemuBuildPCIbridgeDevStr(virDomainPCIbridgeDefPtr dev, + qemuCapsPtr caps, int idx) +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; + +virBufferAsprintf(buf, pci-bridge,chassis_nr=1); The chassis number has to be unique for each bridge. chassis number is not so important, here, I just set all bridges in same chassis. + +if ((dev-type != VIR_DOMAIN_PCIBRIDGE_TYPE_ROOT) +(qemuBuildDeviceAddressStr(buf, dev-info, caps) 0)) +goto error; +else +virBufferAsprintf(buf, ,id=pci-bridge%d , idx); + +if (virBufferError(buf)) { +virReportOOMError(); +goto error; +} + +return virBufferContentAndReset(buf); + +error: +virBufferFreeAndReset(buf); +return NULL; + +} -- regards! li guang -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] conf: Parse and format the new XML
On 2013年01月04日 08:21, Eric Blake wrote: On 01/02/2013 07:37 AM, Osier Yang wrote: Like rawio, sgio is only allowed for block disk of device type lun. It doesn't default disk-sgio to filtered when parsing, as it won't be able to distinguish explicitly requested filtered and a default filtered in driver then. We have to error out for explicit request when the kernel doesn't support the new sysfs knob unpriv_sgio, however, for defaulted filtered, we can just ignore it if the kernel doesn't support unpriv_sgio. --- src/conf/domain_conf.c | 55 +++- src/conf/domain_conf.h | 10 ...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml | 32 +++ tests/qemuxml2xmltest.c|1 + 4 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml +enum virDomainDiskSGIO { +VIR_DOMAIN_DISK_SGIO_DEFAULT = 0, +VIR_DOMAIN_DISK_SGIO_FILTERED, +VIR_DOMAIN_DISK_SGIO_UNFILTERED, + +VIR_DOMAIN_DISK_SGIO_LAST +}; + typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; struct _virDomainBlockIoTuneInfo { unsigned long long total_bytes_sec; @@ -638,6 +646,7 @@ struct _virDomainDiskDef { virStorageEncryptionPtr encryption; bool rawio_specified; int rawio; /* no = 0, yes = 1 */ Don't know why we didn't make this 'bool', but that's pre-existing and would be a separate cleanup patch. +int sgio; I'd add /* enum virDomainDiskSGIO */, to make it easier to see what goes in this int. Okay, will add it when pushing. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] docs: Add docs and rng schema for new XML tag sgio
On 2013年01月04日 05:25, Eric Blake wrote: On 01/02/2013 07:37 AM, Osier Yang wrote: This introduces new XML tag sgio for disk, its valid values are filtered and unfiltered, setting it as filtered will set the disk's unpriv_sgio to 0, and unfiltered to set it as 1, which allows the unprivileged SG_IO commands. --- docs/formatdomain.html.in | 14 ++- docs/schemas/domaincommon.rng | 54 +++- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94df6f8..5e37b92 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1395,7 +1395,19 @@ rawio='yes', rawio capability will be enabled for all disks in the domain (because, in the case of QEMU, this capability can only be set on a per-process basis). This attribute is only -valid when device is lun. +valid when device is lun. NB,coderawio/code intends to +confine the capability per-device, however, current QEMU +implementation gives the domain process broader capability +than that (per-process basis, affects all the domain disks). +To confine the capability as much as possible for QEMU driver +as this stage,codesgio/code is recommended, it's more +secure thancoderawio/code. +The optionalcodesgio/code attribute indicates whether the For consistency with how we did it for 'rawio': The optionalcodesgio/code attribute (span class=sincesince 1.0.2/span) indicates... Okay, +kernel will filter unprivileged SG_IO commands for the disk, +valid settings are filtered or unfiltered. Defaults to +filtered. Same withcoderawio/code,codesgio/code s/Same with/Similar to/ Okay, +is only valid for device 'lun'. +span class=sincesince 1.0.2/span ...then drop thespan here. +group +attribute name=device +choice +valuelun/value +/choice Technically, thechoice isn't needed here (but it doesn't hurt either). ACK with the grammar cleaned up. Will make the change when pushing, thanks. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] util: Prepare helpers for unpriv_sgio setting
On 2013年01月03日 08:01, Eric Blake wrote: On 01/02/2013 07:37 AM, Osier Yang wrote: virGetDeviceID could be used across the sources, but it doesn't relate with this series, and could be done later. * src/util/virutil.h: (Declare virGetDeviceID, and vir{Get,Set}DeviceUnprivSGIO) * src/util/virutil.c: (Implement virGetDeviceID and vir{Get,Set}DeviceUnprivSGIO) * src/libvirt_private.syms: Export private symbols of upper helpers --- src/libvirt_private.syms |3 + src/util/virutil.c | 140 ++ src/util/virutil.h | 11 3 files changed, 154 insertions(+), 0 deletions(-) ACK. +int +virSetDeviceUnprivSGIO(const char *path, + const char *sysfs_dir, + int unpriv_sgio) +{ +char *sysfs_path = NULL; +char *val = NULL; +int ret = -1; +int rc; + +if (!(sysfs_path = virGetUnprivSGIOSysfsPath(path, sysfs_dir))) +return -1; + +if (!virFileExists(sysfs_path)) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(unpriv_sgio is not supported by this kernel)); +goto cleanup; If 'unpriv_sgio' is 0 here, does it make the logic in any later patches easier to return success in that case (you are setting things to the default)? But I'm okay with keeping it as a failure. The unpriv_sgio == 0 could be requested explicitly by user (not only the from the default), and in this case I'd think an error is better. Otherwise I could see one will raise bug like sgio='filtered' succeeded, but sgio='unfiltered' failed unless we document it somewhere. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt unable to start domains
On 01/04/2013 08:30 AM, Doug Goldstein wrote: On Thu, Jan 3, 2013 at 5:58 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: Hi, I updated my libvirt git clone and install yesterday (after about a month or so) and now I can't start domains: $ virsh start fedora18 error: Failed to start domain fedora18 error: unsupported configuration: This QEMU does not support QXL graphics adapters While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila Fedora 18 repo. Just to be clear, qemu was not updated before the problem appeared, only libvirt. Here is the libvirtd log: http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB). Also attaching the domain config. If my `git bisect run` script worked, I've found the commit introducing this issue: 4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit commit 4c993d8ab5473334e9d4155faa913476ada39069 Author: Guannan Ren g...@redhat.com Date: Fri Dec 14 15:06:31 2012 +0800 qemu: add qemu vga devices caps and one cap to mark them usable QEMU_CAPS_DEVICE_QXL -device qxl QEMU_CAPS_DEVICE_VGA -device VGA QEMU_CAPS_DEVICE_CIRRUS_VGA -device cirrus-vga QEMU_CAPS_DEVICE_VMWARE_SVGA -device vmware-svga QEMU_CAPS_DEVICE_VIDEO_PRIMARY /* safe to use -device XXX for primary video device */ Fix a typo in qemuCapsObjectTypes, the string 'qxl' here should be -device qxl rather than -vga [...|qxl|..] :04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758 5afc50956ad09cf6e740ce75800d746a53513271 M src :04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M tests bisect run success FWIW, problem is not reproducible against the parent commit 34ca5684970fc5e4be0ec29809b73dea7be2d84f. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 Basically for some reason we tried to build the command line up as qemu-kvm ... -vga qxl instead of -device qxl-vga which is what I thought Guannan was aiming for. I could be wrong though. I'll take another look at it tonight. When libvirt reports this error, probably the qemu you are using doesn't support qxl-vga as the primary video device, and at the same time the qemu doesn't support -vga qxl either. The reason why libvirt worked out before is that there is a typo for the caps flag bit QEMU_CAPS_VGA_QXL fixed in the above commit. And libvirt didn't set QEMU_CAPS_VGA_QXL on for qemu(1.2) by using QMP mode to check for qemu capabilities. -{ qxl, QEMU_CAPS_VGA_QXL }, +{ qxl, QEMU_CAPS_DEVICE_QXL }, So Doug's patch probably could fix your problem temporarily, but we need to think it through as Eric said. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] qemu: Add a hash table for the shared disks
On 2013年01月04日 04:02, Eric Blake wrote: On 01/02/2013 07:37 AM, Osier Yang wrote: This introduces a hash table for qemu driver, to store the shared disk's info as (@major:minor, @ref_count). @ref_count is the number of domains which shares the disk. Since we only care about if the disk support unprivileged SG_IO commands, and the SG_IO commands only make sense for block disk, this patch only manages (add/remove hash entry) the shared disk for block disk. * src/qemu/qemu_conf.h: (Add member 'sharedDisks' of type virHashTablePtr; Declare helpers qemuGetSharedDiskKey, qemuAddSharedDisk and qemuRemoveSharedDisk) * src/qemu/qemu_conf.c (Implement the 3 helpers) * src/qemu/qemu_process.c (Update 'sharedDisks' when domain starting and shutdown) * src/qemu/qemu_driver.c (Update 'sharedDisks' when attaching or detaching disk). --- src/qemu/qemu_conf.c| 86 +++ src/qemu/qemu_conf.h| 12 ++ src/qemu/qemu_driver.c | 22 src/qemu/qemu_process.c | 15 4 files changed, 135 insertions(+), 0 deletions(-) ACK, as I'd like to see this go in sooner rather than later, and what you have works. However... diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c6deb10..8440247 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -552,3 +552,89 @@ qemuDriverCloseCallbackRunAll(virQEMUDriverPtr driver, virHashForEach(driver-closeCallbacks, qemuDriverCloseCallbackRun,data); } + +/* Construct the hash key for sharedDisks as major:minor */ +char * +qemuGetSharedDiskKey(const char *disk_path) +{ Why are we converting major and minor into a malloc'd char*,... +int major, minor; Here, I'd use 'maj' and 'min' to avoid any shadowing of the major() and minor() macros. Oh, okay, I missed that. +int +qemuAddSharedDisk(virHashTablePtr sharedDisks, + const char *disk_path) +{ +size_t *ref = NULL; +char *key = NULL; + +if (!(key = qemuGetSharedDiskKey(disk_path))) +return -1; + +if ((ref = virHashLookup(sharedDisks, key))) { ...when you could just use a single int value comprising the combination of major and minor as the hash key, if only you had used Sounds good, how about something like 805516, where 8 is major, and 16 is the minor? It should be small enough to be not overflowing. virHashCreateFull() with custom comparators. That would be more efficient (no need to malloc each kay, comparisons are much faster as an integer compare instead of a strcmp, and so on). It may be worth a followup patch that re-does the hash table to be more efficient. +++ b/src/qemu/qemu_driver.c @@ -843,6 +843,9 @@ qemuStartup(bool privileged, if ((qemu_driver-inactivePciHostdevs = pciDeviceListNew()) == NULL) goto error; +if (!(qemu_driver-sharedDisks = virHashCreate(30, NULL))) Here's where the virHashCreateFull would let you avoid having to convert the key into a string. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu
On 2013年01月04日 10:28, li guang wrote: 在 2013-01-03四的 16:13 +0100,Ján Tomko写道: On 12/26/12 02:00, liguang wrote: @@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, * When QEMU grows support for 1 PCI domain, then pci.0 change * to pciNN.0 where NN is the domain number */ -if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) +if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) { +virBufferAsprintf(buf, ,bus=pci-bridge%d, info-addr.pci.bus); +} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) { virBufferAsprintf(buf, ,bus=pci.0); Is there any way (or plan) to use more pci buses with QEMU other than with the pci bridges? If not, we could just name the bridges pci.%d. (If we index the bridges from 1). as far as I know, qemu can't use multi-pci-bus, so only pci.0 accepted now. At this point, I think it's better to ask the QEMU developers to be involved in next series, to make sure things are right. -else +} else { virBufferAsprintf(buf, ,bus=pci); +} if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) virBufferAddLit(buf, ,multifunction=on); else if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF) @@ -3455,6 +3460,32 @@ error: return NULL; } +char * +qemuBuildPCIbridgeDevStr(virDomainPCIbridgeDefPtr dev, + qemuCapsPtr caps, int idx) +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; + +virBufferAsprintf(buf, pci-bridge,chassis_nr=1); The chassis number has to be unique for each bridge. chassis number is not so important, here, I just set all bridges in same chassis. + +if ((dev-type != VIR_DOMAIN_PCIBRIDGE_TYPE_ROOT) +(qemuBuildDeviceAddressStr(buf,dev-info, caps) 0)) +goto error; +else +virBufferAsprintf(buf, ,id=pci-bridge%d , idx); + +if (virBufferError(buf)) { +virReportOOMError(); +goto error; +} + +return virBufferContentAndReset(buf); + +error: +virBufferFreeAndReset(buf); +return NULL; + +} -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt unable to start domains
On 01/04/2013 08:30 AM, Doug Goldstein wrote: On Thu, Jan 3, 2013 at 5:58 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak) zeesha...@gnome.org wrote: Hi, I updated my libvirt git clone and install yesterday (after about a month or so) and now I can't start domains: $ virsh start fedora18 error: Failed to start domain fedora18 error: unsupported configuration: This QEMU does not support QXL graphics adapters While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila Fedora 18 repo. Just to be clear, qemu was not updated before the problem appeared, only libvirt. Here is the libvirtd log: http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB). Also attaching the domain config. If my `git bisect run` script worked, I've found the commit introducing this issue: 4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit commit 4c993d8ab5473334e9d4155faa913476ada39069 Author: Guannan Ren g...@redhat.com Date: Fri Dec 14 15:06:31 2012 +0800 qemu: add qemu vga devices caps and one cap to mark them usable QEMU_CAPS_DEVICE_QXL -device qxl QEMU_CAPS_DEVICE_VGA -device VGA QEMU_CAPS_DEVICE_CIRRUS_VGA -device cirrus-vga QEMU_CAPS_DEVICE_VMWARE_SVGA -device vmware-svga QEMU_CAPS_DEVICE_VIDEO_PRIMARY /* safe to use -device XXX for primary video device */ Fix a typo in qemuCapsObjectTypes, the string 'qxl' here should be -device qxl rather than -vga [...|qxl|..] :04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758 5afc50956ad09cf6e740ce75800d746a53513271 M src :04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M tests bisect run success FWIW, problem is not reproducible against the parent commit 34ca5684970fc5e4be0ec29809b73dea7be2d84f. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 Basically for some reason we tried to build the command line up as qemu-kvm ... -vga qxl instead of -device qxl-vga which is what I thought Guannan was aiming for. I could be wrong though. I'll take another look at it tonight. Actually, qemu(=1.2) will use -device qxl-vga if it supports qxl-vga in its -device output which means the QEMU_CAPS_DEVICE_QXL_VGA is set already. If qemu doesn't support -device qxl-vga even though its version is above 1.2, it will use -vga qxl. For qemu( 1.2), it definitely uses -vga qxl. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ESX:Error] HTTP 403 error for CURL upload operation
I was able to solve this issue, it seems for upload operation the CURL headers needs to contain: Connection: Keep-Alive); Content-Type: application/x-vnd.vmware-streamVmdk); Content-Length: %ld Though the error was related to forbidden access, but it seems it was not correct, with above set of headers and POST operation, VMDK uploads runs smooth :-). Thanks!Ata From: ata.hus...@hotmail.com To: matthias.bo...@googlemail.com Date: Tue, 1 Jan 2013 11:47:13 -0800 CC: libvir-list@redhat.com Subject: Re: [libvirt] [ESX:Error] HTTP 403 error for CURL upload operation The URL /ha-nfc/52c6d592-7636-67c5-29f3-d5b373be4f42/disk-0.vmdk looks like the lease you get during OVA import on the ESX server to upload a disk image to, isn't it? I'm not sure why you would get and Forbidden error from the server here. Are you logged in with an restricted user account instead of the root account? Yes, precicely this is what I am trying to do. The URL (deviceUrl) is obtained once the call to ImportVApp is successful, it does not import the disk on root directory but to the new VM directory created after the importvapp call. I remember trying this on ESXi4.1 and it used to work fine, ESXi5 seems to block it. I am logged in with root credentials. To achieve this operation I've added a routine to support file Upload (for ESX driver as currently it only support buffer upload), I verified its functioning my uploading a file using datastore based URL: (http(s)//ip/file??dcPath=ha-datacenter?dsName=xxx). Are you trying to upload directly to the root of the datastore? IIRC this isn't allowed and you can only upload subdirectories in the datastore as in http(s)//ip/subdirectory/file??dcPath=ha-datacenter?dsName=xxx OVA disk can be compressed (compression algorithm is identified as one of the fields inside creaimportspec result varible), also as OVA optimizes the disk we need to use this URL to upload the valid content and rest necessary messaging is done by server itself (say deflate the disk, uncompress it etc.). -- 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
Re: [libvirt] [PATCH] The -vga CLI arg accepts qxl in qemu 1.2+ always
On 01/04/2013 08:40 AM, Eric Blake wrote: On 01/03/2013 04:42 PM, Doug Goldstein wrote: The -vga command always accepts qxl in 1.2 and newer. Are you sure? I thought qemu ./configure allows a choice of which graphics engines to support, and that you can compile qxl out (distros don't do that, but a self-built qemu might support -vga but not QXL). I think that rather than blindly setting this cap, we need to find the right QMP command to check which graphics engines are supported. When I tried qemu(1.3.50) ./configure --target-list=x86_64-softmmu --enable-debug --disable-spice #./x86_64-softmmu/qemu-system-x86_64 -h|grep -i qxl -vga [std|cirrus|vmware|qxl|xenfb|none] And it seem like no such QMP command for this on my qemu. Guannan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list