- Devel - libvirt List Archives

[libvirt] [PATCH] build: fix 'make check' with pdwtags
by Eric Blake 02 Sep '11

02 Sep '11

Problem introduced by commit b12354b. * src/remote_protocol-structs: Remove spurious blank line. --- Pushing under the build-breaker rule. It only affects people that have pdwtags (from the dwarves package) installed. src/remote_protocol-structs | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 0498bd1..bb5ec44 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1436,7 +1436,6 @@ struct remote_domain_migrate_get_max_speed_args { struct remote_domain_migrate_get_max_speed_ret { uint64_t bandwidth; }; - struct remote_domain_events_register_any_args { int eventID; }; -- 1.7.4.4

1 0

[libvirt] [PATCH] virsh: prefer unsigned flags
by Eric Blake 01 Sep '11

01 Sep '11

* tools/virsh.c (cmdUndefine, cmdSave, cmdSaveImageDumpxml) (cmdSaveImageEdit, cmdManagedSave, cmdRestore, cmdDump) (cmdVcpuPin, cmdSetvcpus, cmdSetmem, cmdSetmaxmem, cmdDumpXML) (cmdDomXMLFromNative, cmdDomXMLToNative, doMigrate) (cmdInterfaceEdit, cmdInterfaceDumpXML, cmdEdit): Match coding style for flags. --- tools/virsh.c | 36 ++++++++++++++++++------------------ 1 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ee5d3bf..c360a20 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1443,7 +1443,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name = NULL; - int flags = 0; + unsigned int flags = 0; int managed_save = vshCommandOptBool(cmd, "managed-save"); int has_managed_save = 0; int rc = -1; @@ -1651,7 +1651,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; const char *to = NULL; bool ret = false; - int flags = 0; + unsigned int flags = 0; const char *xmlfile = NULL; char *xml = NULL; @@ -1713,7 +1713,7 @@ cmdSaveImageDumpxml(vshControl *ctl, const vshCmd *cmd) { const char *file = NULL; bool ret = false; - int flags = 0; + unsigned int flags = 0; char *xml = NULL; if (vshCommandOptBool(cmd, "security-info")) @@ -1810,7 +1810,7 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd) char *tmp = NULL; char *doc = NULL; char *doc_edited = NULL; - int flags = VIR_DOMAIN_XML_SECURE; + unsigned int flags = VIR_DOMAIN_XML_SECURE; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1888,7 +1888,7 @@ cmdManagedSave(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; const char *name; bool ret = false; - int flags = 0; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2246,7 +2246,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd) { const char *from = NULL; bool ret = false; - int flags = 0; + unsigned int flags = 0; const char *xmlfile = NULL; char *xml = NULL; @@ -2309,7 +2309,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; const char *to = NULL; bool ret = false; - int flags = 0; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -3412,7 +3412,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) int live = vshCommandOptBool(cmd, "live"); int current = vshCommandOptBool(cmd, "current"); bool query = false; /* Query mode if no cpulist */ - int flags = 0; + unsigned int flags = 0; if (current) { if (live || config) { @@ -3645,7 +3645,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) int config = vshCommandOptBool(cmd, "config"); int live = vshCommandOptBool(cmd, "live"); int current = vshCommandOptBool(cmd, "current"); - int flags = 0; + unsigned int flags = 0; if (current) { if (live || config) { @@ -3859,7 +3859,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) int config = vshCommandOptBool(cmd, "config"); int live = vshCommandOptBool(cmd, "live"); int current = vshCommandOptBool(cmd, "current"); - int flags = 0; + unsigned int flags = 0; if (current) { if (live || config) { @@ -3948,7 +3948,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) int config = vshCommandOptBool(cmd, "config"); int live = vshCommandOptBool(cmd, "live"); int current = vshCommandOptBool(cmd, "current"); - int flags = VIR_DOMAIN_MEM_MAXIMUM; + unsigned int flags = VIR_DOMAIN_MEM_MAXIMUM; if (current) { if (live || config) { @@ -4612,7 +4612,7 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; char *dump; - int flags = 0; + unsigned int flags = 0; int inactive = vshCommandOptBool(cmd, "inactive"); int secure = vshCommandOptBool(cmd, "security-info"); int update = vshCommandOptBool(cmd, "update-cpu"); @@ -4665,7 +4665,7 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd) const char *configFile = NULL; char *configData; char *xmlData; - int flags = 0; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -4711,7 +4711,7 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) const char *xmlFile = NULL; char *configData; char *xmlData; - int flags = 0; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -4879,7 +4879,7 @@ doMigrate (void *opaque) const char *desturi = NULL; const char *migrateuri = NULL; const char *dname = NULL; - int flags = 0; + unsigned int flags = 0; vshCtrlData *data = opaque; vshControl *ctl = data->ctl; const vshCmd *cmd = data->cmd; @@ -5653,7 +5653,7 @@ cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd) char *doc = NULL; char *doc_edited = NULL; char *doc_reread = NULL; - int flags = VIR_INTERFACE_XML_INACTIVE; + unsigned int flags = VIR_INTERFACE_XML_INACTIVE; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -6194,7 +6194,7 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd) virInterfacePtr iface; bool ret = true; char *dump; - int flags = 0; + unsigned int flags = 0; int inactive = vshCommandOptBool(cmd, "inactive"); if (inactive) @@ -11696,7 +11696,7 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd) char *doc = NULL; char *doc_edited = NULL; char *doc_reread = NULL; - int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; + unsigned int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; -- 1.7.4.4

2 2

[libvirt] [PATCH 0/6] Support online block resizing
by Osier Yang 01 Sep '11

01 Sep '11

Upstream QEMU introduced online block resizing to resize the block device when domain is running since commit 6d4a2b3a4795. It simply reuses existing "bdrv_truncate" method which is used by "qemu-img" to resize the images. This means it supports to resize all the format that "qemu-img resize" supports. E.g. qcow2, raw, qed, qcow, etc. (NB, it doesn't support to shrink size of qcow2 image, and QEMU reports "An undefined error has ocurred" in this case). For block device of virtio bus, the guest can update the device size automatically, needs kernel support, since kernel-2.6.32-117: http://patchwork.usersys.redhat.com/patch/33040/ For block device of SCSI bus, one needs to execute command like below in the guest (linux) to update the size: % echo > /sys/class/scsi_device/0:0:0:0/device/rescan For block device of IDE bus, seems there is no way to let guest update the size after resizing. The new API is defined as: int virDomainBlockResize (virDomainPtr dom, const char *device, unsigned long long size, unsigned int flags) * @device is the name of block device, E.g. vda, vdb. * Units for @size is KB. [PATCH 1/6] block_resize: Define the new API [PATCH 2/6] block_resize: Implemente the public API [PATCH 3/6] block_resize: Wire up the remote protocol [PATCH 4/6] block_resize: Implemente the qemu monitor functions [PATCH 5/6] block_resize: Implemente the internal API for qemu [PATCH 6/6] block_resize: Expose the new API in virsh Regards, Osier

2 9

[libvirt] [v2 00/13] various USB support improvements
by Marc-André Lureau 01 Sep '11

01 Sep '11

Hola, This is the second version of the patch set which should address most of the comments I recieved. It should be good enough to let virt-manager & co support USB2 & usb-redirection. Some of the auto-assign of USB addresses etc.. are leftover for future improvements. It should not introduce regressions. Marc-André Lureau (13): Add various USB devices QEMU_CAPS Split virDomainControllerModel to virDomainControllerModelSCSI Add USB controller models Add a new controller type 'usb' with optionnal 'model' USB controller can have a PCI address child element USB devices gain a new USB address child element Add USB companion controllers support Add USB hub device Modify USB port to be defined as a port path qemu: don't reserve slot 1 if a PIIX3 USB controller is defined there qemu: Don't append 0 at usb id, so that it is compatible with legacy -usb Add a usb1 & usb2 qemuxml2argv test Add usb-redir device docs/formatdomain.html.in | 81 ++++- docs/schemas/domain.rng | 122 +++++-- src/conf/domain_conf.c | 369 +++++++++++++++++++- src/conf/domain_conf.h | 82 ++++- src/esx/esx_driver.c | 8 +- src/libvirt_private.syms | 8 +- src/qemu/qemu_capabilities.c | 28 ++ src/qemu/qemu_capabilities.h | 9 + src/qemu/qemu_cgroup.c | 3 +- src/qemu/qemu_command.c | 284 ++++++++++++++-- src/qemu/qemu_command.h | 10 +- src/qemu/qemu_hostdev.c | 2 + src/qemu/qemu_hotplug.c | 66 ++++- src/qemu/qemu_hotplug.h | 3 + src/security/security_dac.c | 6 + src/security/security_selinux.c | 6 + src/vmx/vmx.c | 32 +- tests/qemuhelptest.c | 17 +- .../qemuxml2argv-input-usbmouse-addr.args | 1 + .../qemuxml2argv-input-usbmouse-addr.xml | 27 ++ .../qemuxml2argv-usb-controller.args | 1 + .../qemuxml2argv-usb-controller.xml | 16 + tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml | 19 + .../qemuxml2argv-usb-ich9-companion.args | 6 + .../qemuxml2argv-usb-ich9-companion.xml | 30 ++ .../qemuxml2argv-usb-ich9-ehci-addr.args | 1 + .../qemuxml2argv-usb-ich9-ehci-addr.xml | 18 + .../qemuxml2argv-usb-piix3-controller.args | 1 + .../qemuxml2argv-usb-piix3-controller.xml | 16 + tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml | 31 ++ tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 8 + tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 33 ++ tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args | 15 + tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 74 ++++ tests/qemuxml2argvtest.c | 29 ++ tests/xml2vmxtest.c | 2 +- 38 files changed, 1350 insertions(+), 116 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml -- 1.7.6

2 26

[libvirt] [PATCH] Source control for storage pool
by Lei Li 01 Sep '11

01 Sep '11

Fix bug #611823 storage driver should prohibit pools with duplicate underlying storage. Add API virStoragePoolSourceFindDuplicate() to do uniqueness check based on source location infomation for pool type. Signed-off-by: Lei Li <lilei(a)linux.vnet.ibm.com> --- src/conf/storage_conf.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/conf/storage_conf.h | 3 ++ src/libvirt_private.syms | 1 + src/storage/storage_driver.c | 6 +++ 4 files changed, 83 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8d14e87..698334e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1701,6 +1701,79 @@ cleanup: return ret; } +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ + int i, j, k; + int ret = 1; + virStoragePoolObjPtr pool = NULL; + virStoragePoolObjPtr matchpool = NULL; + + /* Check the pool list for duplicate underlying storage */ + for (i = 0; i < pools->count; i++) { + pool = pools->objs[i]; + if (def->type != pool->def->type) + continue; + + virStoragePoolObjLock(pool); + if (pool->def->type == VIR_STORAGE_POOL_DIR) { + if (STREQ(pool->def->target.path, def->target.path)) { + matchpool = pool; + goto cleanup; + } + } else if (pool->def->type == VIR_STORAGE_POOL_NETFS) { + if ((STREQ(pool->def->source.dir, def->source.dir)) \ + && (STREQ(pool->def->source.host.name, def->source.host.name))) { + matchpool = pool; + goto cleanup; + } + } else if (pool->def->type == VIR_STORAGE_POOL_SCSI) { + if (STREQ(pool->def->source.adapter, def->source.adapter)) { + matchpool = pool; + goto cleanup; + } + } else if (pool->def->type == VIR_STORAGE_POOL_ISCSI) { + for (j = 0; j < pool->def->source.ndevice; j++) { + for (k = 0; k < def->source.ndevice; k++) { + if (pool->def->source.initiator.iqn) { + if ((STREQ(pool->def->source.devices[j].path, def->source.devices[k].path)) \ + && (STREQ(pool->def->source.initiator.iqn, def->source.initiator.iqn))) { + matchpool = pool; + goto cleanup; + } + } else if (pool->def->source.host.name) { + if ((STREQ(pool->def->source.devices[j].path, def->source.devices[k].path)) \ + && (STREQ(pool->def->source.host.name, def->source.host.name))) { + matchpool = pool; + goto cleanup; + } + } + } + } + } else { + /* For the remain three pool type 'fs''logical''disk' that all use device path */ + if (pool->def->source.ndevice) { + for (j = 0; j < pool->def->source.ndevice; j++) { + for (k = 0; k < def->source.ndevice; k++) { + if (STREQ(pool->def->source.devices[j].path, def->source.devices[k].path)) { + matchpool = pool; + goto cleanup; + } + } + } + } + } + virStoragePoolObjUnlock(pool); + } +cleanup: + if (matchpool) { + virStoragePoolObjUnlock(matchpool); + virStorageReportError(VIR_ERR_OPERATION_FAILED, + _("pool source location info is duplicate")); + ret = -1; + } + return ret; +} void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 271441a..a8562a6 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -388,6 +388,9 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, unsigned int check_active); +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def); + void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f03e30..5899d56 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -948,6 +948,7 @@ virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; virStoragePoolObjIsDuplicate; +virStoragePoolSourceFindDuplicate; virStoragePoolObjListFree; virStoragePoolObjLock; virStoragePoolObjRemove; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 68cac1f..c05b74e 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -536,6 +536,9 @@ storagePoolCreate(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(&driver->pools, def, 1) < 0) goto cleanup; + if (virStoragePoolSourceFindDuplicate(&driver->pools, def) < 0) + goto cleanup; + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; @@ -589,6 +592,9 @@ storagePoolDefine(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(&driver->pools, def, 0) < 0) goto cleanup; + if (virStoragePoolSourceFindDuplicate(&driver->pools, def) < 0) + goto cleanup; + if (virStorageBackendForType(def->type) == NULL) goto cleanup; -- 1.7.1

2 1

[libvirt] [PATCH] qemu: Fix a regression of domain save
by Osier Yang 01 Sep '11

01 Sep '11

* src/qemu/qemu_driver.c - qemuDomainSaveInternal: Return directly will keep the domain object locked. --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4e8c691..a150b08 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2223,7 +2223,7 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom, if (qemuProcessAutoDestroyActive(driver, vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is marked for auto destroy")); - return -1; + goto cleanup; } memset(&header, 0, sizeof(header)); -- 1.7.6

2 3

[libvirt] [PATCH 0/8] Cleanup improper VIR_ERR_NO_SUPPORT use
by Osier Yang 01 Sep '11

01 Sep '11

Error code VIR_ERR_NO_SUPPORT will be translated to "this function is not supported by the connection driver", however, it's used across the whole projects, this patch is trying to cleanup all the improper use in the source tree. The modification can be grouped to 3 following groups: 1) The error intends to tell user it's invalid operation. s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID. 2) The error intends to tell the configuration of domain is not supported. s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/ 3) The error intends to tell the function is not implemented on some platform. * s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/ * and add error strings e.g. static int lxcDomainInterfaceStats(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED, struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED) -lxcError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +lxcError(VIR_ERR_OPERATION_INVALID, "%s", + _("interface stats not implemented on this platform")); return -1; } [PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in [PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use [PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use [PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use [PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use [PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use [PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use Regards Osier

4 43

[libvirt] [PATCH] Fix tracking of RPC messages wrt streams
by Daniel P. Berrange 01 Sep '11

01 Sep '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> Commit 2c85644b0b51fbe5b6244e6773531af29933a727 attempted to fix a problem with tracking RPC messages from streams by doing - if (msg->header.type == VIR_NET_REPLY) { + if (msg->header.type == VIR_NET_REPLY || + (msg->header.type == VIR_NET_STREAM && + msg->header.status != VIR_NET_CONTINUE)) { client->nrequests--; In other words any stream packet, with status NET_OK or NET_ERROR would cause nrequests to be decremented. This is great if the packet from from a synchronous virStreamFinish or virStreamAbort API call, but wildly wrong if from a server initiated abort. The latter resulted in 'nrequests' being decremented below zero. This then causes all I/O for that client to be stopped. Instead of trying to infer whether we need to decrement the nrequests field, from the message type/status, introduce an explicit 'bool tracked' field to mark whether the virNetMessagePtr object is subject to tracking. Also add a virNetMessageClear function to allow a message contents to be cleared out, without adversely impacting the 'tracked' field as a naive memset() would do * src/rpc/virnetmessage.c, src/rpc/virnetmessage.h: Add a 'bool tracked' field and virNetMessageClear() API * daemon/remote.c, daemon/stream.c, src/rpc/virnetclientprogram.c, src/rpc/virnetclientstream.c, src/rpc/virnetserverclient.c, src/rpc/virnetserverprogram.c: Switch over to use virNetMessageClear() and pass in the 'bool tracked' value when creating messages. --- daemon/remote.c | 2 +- daemon/stream.c | 10 +++++----- src/rpc/virnetclientprogram.c | 2 +- src/rpc/virnetclientstream.c | 4 ++-- src/rpc/virnetmessage.c | 14 ++++++++++++-- src/rpc/virnetmessage.h | 6 +++++- src/rpc/virnetserverclient.c | 12 +++++------- src/rpc/virnetserverprogram.c | 2 +- 8 files changed, 32 insertions(+), 20 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 34c6364..d5ead81 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2495,7 +2495,7 @@ remoteDispatchDomainEventSend(virNetServerClientPtr client, { virNetMessagePtr msg; - if (!(msg = virNetMessageNew())) + if (!(msg = virNetMessageNew(false))) goto cleanup; msg->header.prog = virNetServerProgramGetID(program); diff --git a/daemon/stream.c b/daemon/stream.c index ba3adc2..e3214c2 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -207,7 +207,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) virNetError(VIR_ERR_RPC, "%s", _("stream had I/O failure")); - msg = virNetMessageNew(); + msg = virNetMessageNew(false); if (!msg) { ret = -1; } else { @@ -344,7 +344,7 @@ int daemonFreeClientStream(virNetServerClientPtr client, virNetMessagePtr tmp = msg->next; if (client) { /* Send a dummy reply to free up 'msg' & unblock client rx */ - memset(msg, 0, sizeof(*msg)); + virNetMessageClear(msg); msg->header.type = VIR_NET_REPLY; if (virNetServerClientSendMessage(client, msg) < 0) { virNetServerClientImmediateClose(client); @@ -653,7 +653,7 @@ daemonStreamHandleWrite(virNetServerClientPtr client, * its active request count / throttling */ if (msg->header.status == VIR_NET_CONTINUE) { - memset(msg, 0, sizeof(*msg)); + virNetMessageClear(msg); msg->header.type = VIR_NET_REPLY; if (virNetServerClientSendMessage(client, msg) < 0) { virNetMessageFree(msg); @@ -715,7 +715,7 @@ daemonStreamHandleRead(virNetServerClientPtr client, memset(&rerr, 0, sizeof(rerr)); - if (!(msg = virNetMessageNew())) + if (!(msg = virNetMessageNew(false))) ret = -1; else ret = virNetServerProgramSendStreamError(remoteProgram, @@ -729,7 +729,7 @@ daemonStreamHandleRead(virNetServerClientPtr client, stream->tx = 0; if (ret == 0) stream->recvEOF = 1; - if (!(msg = virNetMessageNew())) + if (!(msg = virNetMessageNew(false))) ret = -1; if (msg) { diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c index c39520a..a07b744 100644 --- a/src/rpc/virnetclientprogram.c +++ b/src/rpc/virnetclientprogram.c @@ -272,7 +272,7 @@ int virNetClientProgramCall(virNetClientProgramPtr prog, { virNetMessagePtr msg; - if (!(msg = virNetMessageNew())) + if (!(msg = virNetMessageNew(false))) return -1; msg->header.prog = prog->program; diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index fe15acd..2cc84d4 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -328,7 +328,7 @@ int virNetClientStreamSendPacket(virNetClientStreamPtr st, bool wantReply; VIR_DEBUG("st=%p status=%d data=%p nbytes=%zu", st, status, data, nbytes); - if (!(msg = virNetMessageNew())) + if (!(msg = virNetMessageNew(false))) return -1; virMutexLock(&st->lock); @@ -390,7 +390,7 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr st, goto cleanup; } - if (!(msg = virNetMessageNew())) { + if (!(msg = virNetMessageNew(false))) { virReportOOMError(); goto cleanup; } diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index 0725491..a1ae9c4 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -32,7 +32,7 @@ virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) -virNetMessagePtr virNetMessageNew(void) +virNetMessagePtr virNetMessageNew(bool tracked) { virNetMessagePtr msg; @@ -41,11 +41,21 @@ virNetMessagePtr virNetMessageNew(void) return NULL; } - VIR_DEBUG("msg=%p", msg); + msg->tracked = tracked; + VIR_DEBUG("msg=%p tracked=%d", msg, tracked); return msg; } + +void virNetMessageClear(virNetMessagePtr msg) +{ + bool tracked = msg->tracked; + memset(msg, 0, sizeof(*msg)); + msg->tracked = tracked; +} + + void virNetMessageFree(virNetMessagePtr msg) { if (!msg) diff --git a/src/rpc/virnetmessage.h b/src/rpc/virnetmessage.h index 2aae3f6..307a041 100644 --- a/src/rpc/virnetmessage.h +++ b/src/rpc/virnetmessage.h @@ -35,6 +35,8 @@ typedef void (*virNetMessageFreeCallback)(virNetMessagePtr msg, void *opaque); * use virNetMessageNew() to allocate on the heap */ struct _virNetMessage { + bool tracked; + char buffer[VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX]; size_t bufferLength; size_t bufferOffset; @@ -48,7 +50,9 @@ struct _virNetMessage { }; -virNetMessagePtr virNetMessageNew(void); +virNetMessagePtr virNetMessageNew(bool tracked); + +void virNetMessageClear(virNetMessagePtr); void virNetMessageFree(virNetMessagePtr msg); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index a73b06d..412814d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -277,7 +277,7 @@ virNetServerClientCheckAccess(virNetServerClientPtr client) return -1; } - if (!(confirm = virNetMessageNew())) + if (!(confirm = virNetMessageNew(false))) return -1; /* Checks have succeeded. Write a '\1' byte back to the client to @@ -323,7 +323,7 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, virNetTLSContextRef(tls); /* Prepare one for packet receive */ - if (!(client->rx = virNetMessageNew())) + if (!(client->rx = virNetMessageNew(true))) goto error; client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX; client->nrequests = 1; @@ -805,7 +805,7 @@ readmore: /* Possibly need to create another receive buffer */ if (client->nrequests < client->nrequests_max) { - if (!(client->rx = virNetMessageNew())) { + if (!(client->rx = virNetMessageNew(true))) { client->wantClose = true; } else { client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX; @@ -885,16 +885,14 @@ virNetServerClientDispatchWrite(virNetServerClientPtr client) /* Get finished msg from head of tx queue */ msg = virNetMessageQueueServe(&client->tx); - if (msg->header.type == VIR_NET_REPLY || - (msg->header.type == VIR_NET_STREAM && - msg->header.status != VIR_NET_CONTINUE)) { + if (msg->tracked) { client->nrequests--; /* See if the recv queue is currently throttled */ if (!client->rx && client->nrequests < client->nrequests_max) { /* Ready to recv more messages */ + virNetMessageClear(msg); client->rx = msg; - memset(client->rx, 0, sizeof(*client->rx)); client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX; msg = NULL; client->nrequests++; diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 2e9e3f7..643a97d 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -284,7 +284,7 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog, VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d status=%d", msg->header.serial, msg->header.proc, msg->header.status); /* Send a dummy reply to free up 'msg' & unblock client rx */ - memset(msg, 0, sizeof(*msg)); + virNetMessageClear(msg); msg->header.type = VIR_NET_REPLY; if (virNetServerClientSendMessage(client, msg) < 0) { ret = -1; -- 1.7.6

4 3

[libvirt] [PATCH] Avoid use-after-free on streams, due to message callbacks
by Daniel P. Berrange 01 Sep '11

01 Sep '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> When sending outbound stream RPC messages, a callback is used to re-enable stream data transmission. If the stream aborts while one of these messages is outstanding, the stream may have been free'd by the time it is invoked. This results in a use-after-free error * daemon/stream.c: Ref-count streams to avoid use-after-free --- daemon/stream.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/daemon/stream.c b/daemon/stream.c index 7d2b367..ba3adc2 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -38,6 +38,7 @@ struct daemonClientStream { daemonClientPrivatePtr priv; + int refs; virNetServerProgramPtr prog; @@ -102,6 +103,8 @@ daemonStreamMessageFinished(virNetMessagePtr msg, stream->tx = 1; daemonStreamUpdateEvents(stream); + + daemonFreeClientStream(NULL, stream); } @@ -299,6 +302,7 @@ daemonCreateClientStream(virNetServerClientPtr client, return NULL; } + stream->refs = 1; stream->priv = priv; stream->prog = prog; stream->procedure = header->proc; @@ -326,6 +330,10 @@ int daemonFreeClientStream(virNetServerClientPtr client, if (!stream) return 0; + stream->refs--; + if (stream->refs) + return 0; + VIR_DEBUG("client=%p, proc=%d, serial=%d", client, stream->procedure, stream->serial); @@ -727,7 +735,7 @@ daemonStreamHandleRead(virNetServerClientPtr client, if (msg) { msg->cb = daemonStreamMessageFinished; msg->opaque = stream; - virNetServerClientRef(client); + stream->refs++; ret = virNetServerProgramSendStreamData(remoteProgram, client, msg, -- 1.7.6

2 1

[libvirt] Python stream callback removal
by Dave Allan 01 Sep '11

01 Sep '11

I'm trying to write an example serial console implementation in python (attached), but I'm having some trouble getting stream events to do what I want. The console itself works fine as long as the domain stays up, but as soon as the domain shuts down the python script goes into a tight loop repeatedly calling the stream event callback. Debugging indicates that the stream event callback is being requested to be removed, but it never actually is removed which makes me think I am not properly releasing some resource, but I was under the impression that an error on a stream resulting in the stream aborting was supposed to free all the resources for me. Is that not correct? Thanks for any help anybody can provide, Dave

3 9