[libvirt] [PATCH] free buf->content when vsnprintf() failed
When buf->error is 1, we do not return buf->content in the function
virBufferContentAndReset(). So we should free buf->content when
vsnprintf() failed.
---
src/util/buf.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/util/buf.c b/src/util/buf.c
index 7557ad1..fdb7660 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -241,6 +241,9 @@ virBufferVSprintf(const virBufferPtr buf, const char
*format, ...)
size = buf->size - buf->use;
if ((count = vsnprintf(&buf->content[buf->use],
size, format, argptr)) < 0) {
+VIR_FREE(buf->content);
+buf->size = 0;
+buf->use = 0;
buf->error = 1;
goto err;
}
@@ -259,6 +262,9 @@ virBufferVSprintf(const virBufferPtr buf, const char
*format, ...)
size = buf->size - buf->use;
if ((count = vsnprintf(&buf->content[buf->use],
size, format, argptr)) < 0) {
+VIR_FREE(buf->content);
+buf->size = 0;
+buf->use = 0;
buf->error = 1;
goto err;
}
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] release PCI address only when we have ensured it successfully
Steps to reproduce this bug:
1. # cat net.xml # 00:03.0 has been used
2. # virsh attach-device vm1 net.xml
error: Failed to attach device from net.xml
error: internal error unable to reserve PCI address 0:0:3
3. # virsh attach-device vm1 net.xml
error: Failed to attach device from net.xml
error: internal error unable to execute QEMU command 'device_add': Device
'rtl8139' could not be initialized
The reason of this bug is that: we can not reserve PCI address 0:0:3 because it
has
been used, but we release PCI address when we reserve it failed.
---
src/qemu/qemu_hotplug.c | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b03f774..5fdb013 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -147,6 +147,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver
*driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
char *devstr = NULL;
char *drivestr = NULL;
+bool releaseaddr = false;
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -163,6 +164,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver
*driver,
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0)
goto error;
+releaseaddr = true;
if (qemuAssignDeviceDiskAlias(disk, qemuCaps) < 0)
goto error;
@@ -221,6 +223,7 @@ error:
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
(disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+releaseaddr &&
qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0)
VIR_WARN("Unable to release PCI address on %s", disk->src);
@@ -242,6 +245,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver
*driver,
const char* type = virDomainControllerTypeToString(controller->type);
char *devstr = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
+bool releaseaddr = false;
for (i = 0 ; i < vm->def->ncontrollers ; i++) {
if ((vm->def->controllers[i]->type == controller->type) &&
@@ -256,6 +260,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver
*driver,
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info)
< 0)
goto cleanup;
+releaseaddr = true;
if (qemuAssignDeviceControllerAlias(controller) < 0)
goto cleanup;
@@ -288,6 +293,7 @@ cleanup:
if ((ret != 0) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
(controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+releaseaddr &&
qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &controller->info) < 0)
VIR_WARN0("Unable to release PCI address on controller");
@@ -559,6 +565,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
int ret = -1;
virDomainDevicePCIAddress guestAddr;
int vlan;
+bool releaseaddr = false;
if (!qemuCapsGet(qemuCaps, QEMU_CAPS_HOST_NET_ADD)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -595,6 +602,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0)
goto cleanup;
+releaseaddr = true;
+
if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
vlan = -1;
@@ -694,6 +703,7 @@ cleanup:
if ((ret != 0) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
(net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+releaseaddr &&
qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0)
VIR_WARN0("Unable to release PCI address on NIC");
@@ -757,6 +767,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver
*driver,
char *devstr = NULL;
int configfd = -1;
char *configfd_name = NULL;
+bool releaseaddr = false;
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) {
virReportOOMError();
@@ -771,6 +782,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver
*driver,
goto error;
if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0)
goto error;
+releaseaddr = true;
if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) {
configfd = qemuOpenPCIConfig(hostdev);
if (configfd >= 0) {
@@ -823,6 +835,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver
*driver,
error:
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
(hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+releaseaddr &&
qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &hostdev->info) < 0)
VIR_WARN0("Unable to release PCI address on host devi
[libvirt] [PATCH] fix virsh's regression
This patch does the following things:
1. The return value of cmdSchedInfoUpdate() can be -1, 0 and 1. So the
type of return value should be int not bool.(This function is not a
entry of a virsh command, but the name of this function likes cmdXXX)
2. The type of cmdSchedinfo()'s, cmdFreecell()'s, cmdPoolList()'s and
cmdVolList()'s return value is bool not int, so change the type of
variable ret_val, func_ret and functionReturn.
3. Add a variable functionReturn for cmdMigrate(), cmdAttachInterface(),
cmdDetachInterface(), cmdAttachDisk() and cmdDetachDisk() to save the
return value.
4. Change the type of variable ret in the function cmdAttachDevice(),
cmdDetachDevice(), cmdUpdateDevice(), cmdAttachInterface(),
cmdDetachInterface(), cmdAttachDisk() and cmdDetachDisk() to int, as
we use it to save the return value of virXXX() and the type of virXXX()'s
return value is int not bool.
5. Do some cleanup when virBuff.error is 1.
The bug 1-4 were introduced by commit b56fa5bb.
---
tools/virsh.c | 63 +---
1 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 9ac27b3..27140f3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1593,7 +1593,7 @@ static const vshCmdOptDef opts_schedinfo[] = {
{NULL, 0, 0, NULL}
};
-static bool
+static int
cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
virSchedParameterPtr param)
{
@@ -1696,7 +1696,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
int nparams = 0;
int update = 0;
int i, ret;
-int ret_val = false;
+bool ret_val = false;
if (!vshConnectionUsability(ctl, ctl->conn))
return false;
@@ -2288,7 +2288,7 @@ static const vshCmdOptDef opts_freecell[] = {
static bool
cmdFreecell(vshControl *ctl, const vshCmd *cmd)
{
-int func_ret = false;
+bool func_ret = false;
int ret;
int cell = -1, cell_given;
unsigned long long memory;
@@ -3847,6 +3847,7 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom = NULL;
int p[2] = {-1, -1};
int ret = -1;
+bool functionReturn = false;
virThread workerThread;
struct pollfd pollfd;
char retchar;
@@ -3921,15 +3922,15 @@ repoll:
if (ret > 0) {
if (saferead(p[0], &retchar, sizeof(retchar)) > 0) {
if (retchar == '0') {
-ret = true;
+functionReturn = true;
if (verbose) {
/* print [100 %] */
print_job_progress(0, 1);
}
} else
-ret = false;
+functionReturn = false;
} else
-ret = false;
+functionReturn = false;
break;
}
@@ -3937,11 +3938,11 @@ repoll:
if (errno == EINTR) {
if (intCaught) {
virDomainAbortJob(dom);
-ret = false;
intCaught = 0;
} else
goto repoll;
}
+functionReturn = false;
break;
}
@@ -3975,7 +3976,7 @@ cleanup:
virDomainFree(dom);
VIR_FORCE_CLOSE(p[0]);
VIR_FORCE_CLOSE(p[1]);
-return ret;
+return functionReturn;
}
/*
@@ -5952,7 +5953,8 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
{
virStoragePoolInfo info;
char **poolNames = NULL;
-int i, functionReturn, ret;
+int i, ret;
+bool functionReturn;
int numActivePools = 0, numInactivePools = 0, numAllPools = 0;
size_t stringLength = 0, nameStrLength = 0;
size_t autostartStrLength = 0, persistStrLength = 0;
@@ -7525,7 +7527,8 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
double val;
int details = vshCommandOptBool(cmd, "details");
int numVolumes = 0, i;
-int ret, functionReturn;
+int ret;
+bool functionReturn;
int stringLength = 0;
size_t allocStrLength = 0, capStrLength = 0;
size_t nameStrLength = 0, pathStrLength = 0;
@@ -8904,7 +8907,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
const char *from = NULL;
char *buffer;
-bool ret = true;
+int ret;
unsigned int flags;
if (!vshConnectionUsability(ctl, ctl->conn))
@@ -8969,7 +8972,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
const char *from = NULL;
char *buffer;
-bool ret = true;
+int ret;
unsigned int flags;
if (!vshConnectionUsability(ctl, ctl->conn))
@@ -9035,7 +9038,7 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom;
const char *from = NULL;
char *buffer;
-bool ret = true;
+int ret;
unsigned int flags;
if (!vshConnectionUsability(ctl, ctl->conn))
@@ -9110,7 +91
Re: [libvirt] [PATCHv12 1/3] libvirt/qemu - support persistent modification of devices
On Fri, 22 Apr 2011 12:07:56 +0900
KAMEZAWA Hiroyuki wrote:
>
> Rebased ont the latest git tree, which makes this work easier.
> This series adds support for attach/detach/update disks of domain config.
Ping ?
Thanks,
-Kame
> ==
> This patch adds functions for modify domain's persistent definition.
> To do error recovery in easy way, we use a copy of vmdef and update it.
>
> The whole sequence will be:
>
> make a copy of domain definition.
>
> if (flags & MODIFY_CONFIG)
> update copied domain definition
> if (flags & MODIF_LIVE)
> do hotplug.
> if (no error)
> save copied one to the file and update cached definition.
> else
> discard copied definition.
>
> This patch is mixuture of Eric Blake's work and mine.
> From: Eric Blake
> Signed-off-by: KAMEZAWA Hiroyuki
>
> Changelog: v11 -> v12
> - rebased and fixed hunks.
> - renamed qemudDomainto qemuDomain...
>
> (virDomainObjCopyPersistentDef): make a copy of persistent vm definition
> (qemuDomainAttach/Detach/UpdateDeviceConfig) : callbacks. now empty
> (qemuDomainModifyDeviceFlags): add support for MODIFY_CONFIG and
> MODIFY_CURRENT
> ---
> src/conf/domain_conf.c | 18 ++
> src/conf/domain_conf.h |3 +
> src/libvirt_private.syms |1 +
> src/qemu/qemu_driver.c | 147
> --
> 4 files changed, 137 insertions(+), 32 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 381e692..6c1098a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9509,3 +9509,21 @@ cleanup:
>
> return ret;
> }
> +
> +
> +virDomainDefPtr
> +virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom)
> +{
> +char *xml;
> +virDomainDefPtr cur, ret;
> +
> +cur = virDomainObjGetPersistentDef(caps, dom);
> +
> +xml = virDomainDefFormat(cur, VIR_DOMAIN_XML_WRITE_FLAGS);
> +if (!xml)
> +return NULL;
> +
> +ret = virDomainDefParseString(caps, xml, VIR_DOMAIN_XML_READ_FLAGS);
> +
> +return ret;
> +}
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 6ea30b9..ddf111a 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1288,6 +1288,9 @@ int virDomainObjSetDefTransient(virCapsPtr caps,
> virDomainDefPtr
> virDomainObjGetPersistentDef(virCapsPtr caps,
> virDomainObjPtr domain);
> +virDomainDefPtr
> +virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom);
> +
> void virDomainRemoveInactive(virDomainObjListPtr doms,
> virDomainObjPtr dom);
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ba7739d..f732431 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -287,6 +287,7 @@ virDomainMemballoonModelTypeToString;
> virDomainNetDefFree;
> virDomainNetTypeToString;
> virDomainObjAssignDef;
> +virDomainObjCopyPersistentDef;
> virDomainObjSetDefTransient;
> virDomainObjGetPersistentDef;
> virDomainObjIsDuplicate;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 771678e..fd85c8a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4073,6 +4073,46 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
> return ret;
> }
>
> +static int
> +qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED,
> + virDomainDeviceDefPtr dev)
> +{
> +switch (dev->type) {
> +default:
> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("persistent attach of device is not supported"));
> + return -1;
> +}
> +return 0;
> +}
> +
> +
> +static int
> +qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED,
> + virDomainDeviceDefPtr dev)
> +{
> +switch (dev->type) {
> +default:
> +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +_("persistent detach of device is not supported"));
> +return -1;
> +}
> +return 0;
> +}
> +
> +static int
> +qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED,
> + virDomainDeviceDefPtr dev)
> +{
> +switch (dev->type) {
> +default:
> +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("persistent update of device is not supported"));
> +return -1;
> +}
> +return 0;
> +}
> +
> /* Actions for qemuDomainModifyDeviceFlags */
> enum {
> QEMU_DEVICE_ATTACH,
> @@ -4088,6 +4128,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const
> char *xml,
> struct qemud_driver *driver = dom->conn->privateData;
> virBitmapPtr qemuCaps = NULL;
> virDomainObjPtr vm = NULL;
> +virDomainDefPtr vmdef = NULL;
> virDomainDeviceDefPtr dev = NULL;
> bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0;
> int ret = -1;
> @@ -4097,12 +4138
[libvirt] [PATCH] free memory properly in cleanup patch
virsh schedinfo inactive-domain will trigger the problem.
---
daemon/remote.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 1c98bba..eedbc77 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -945,8 +945,11 @@ remoteDispatchDomainGetSchedulerParameters(struct
qemud_server *server ATTRIBUTE
cleanup:
if (rv < 0) {
remoteDispatchError(rerr);
-for (i = 0 ; i < nparams ; i++)
-VIR_FREE(ret->params.params_val[i].field);
+if (ret->params.params_val) {
+for (i = 0 ; i < nparams ; i++)
+VIR_FREE(ret->params.params_val[i].field);
+VIR_FREE(ret->params.params_val);
+}
}
if (dom)
virDomainFree(dom);
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [virt-tools-list] libvirt spice command line appears to be incorrect
On Tue, Apr 26, 2011 at 2:05 AM, Eric Blake wrote: > On 04/25/2011 03:54 PM, Emre Erenoglu wrote: > > yes, libvirt 0.8.7 was silently reverting the change I made manually in > the > > xml file. Neither virsh edit nor editing the xml and re-defining it > worked. > > Once libvirt saw the "spicevmc" there, it just removed it and put "null" > > instead. > > There has been some upstream work to make libvirt do better at detecting > bogus configurations, but I'm not sure off the top of my head if it > includes the instance you tripped over. So, the question remains > whether we have already fixed the bug in 0.9.0, or whether, if you put > in some other random string in place of "spicevmc", then would libvirt > still silently change that to "null" instead of rejecting the XML. If > the former, great - we've cleaned it up! If the latter, then this is a > bug still in libvirt worth fixing. > > I re-installed 0.8.7. It re-wrote the existing virtual machine channel device definition (which used to be "spicevmc" with 0.9.0) back to 'null'. I tried to edit it by hand, with any string, always reverted to 'null'. In libvirt 0.9.0, editing by hand to replace 'null' with 'spicevmc' works. Editing by hand to an arbitrary string, fails with the following error message (when I put 'emre' instead of 'null'): error: XML description for unknown type presented to host for character device: emre is not well formed or invalid So looks like it's fixed in 0.9.0 but I let you conclude the 0.8.7 behaviour. -- Emre -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [virt-tools-list] libvirt spice command line appears to be incorrect
On 04/25/2011 03:54 PM, Emre Erenoglu wrote: > yes, libvirt 0.8.7 was silently reverting the change I made manually in the > xml file. Neither virsh edit nor editing the xml and re-defining it worked. > Once libvirt saw the "spicevmc" there, it just removed it and put "null" > instead. There has been some upstream work to make libvirt do better at detecting bogus configurations, but I'm not sure off the top of my head if it includes the instance you tripped over. So, the question remains whether we have already fixed the bug in 0.9.0, or whether, if you put in some other random string in place of "spicevmc", then would libvirt still silently change that to "null" instead of rejecting the XML. If the former, great - we've cleaned it up! If the latter, then this is a bug still in libvirt worth fixing. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [virt-tools-list] libvirt spice command line appears to be incorrect
On Mon, Apr 25, 2011 at 6:28 PM, Cole Robinson wrote: > On 04/25/2011 04:27 AM, Richard W.M. Jones wrote: > > [Copying this to libvir-list] > > > > On Mon, Apr 25, 2011 at 01:07:37AM +0400, Emre Erenoglu wrote: > >> Hi, > >> > >> I'm the package maintainer for virt-manager and related packages for > Pardus > >> distribution. While testing the latest libvirt, virtinst & virt-manager > >> packages, I've come across a strange issue and I would like to get your > >> valuable opinion. > >> > >> I add all spice related devices and everything works good, except the > >> vdagent inside the windows xp guest. The virtio serial driver is loaded > >> correctly. As I track down the issue, I found out that libvirt is > starting > >> qemu-kvm with parameters which do not match the ones adviced by the > spice > >> people. Please see below email discussion with them on this. The > offending > >> line seems to be the chardev parameter. qemu-kvm is started by > virt-manager > >> with the following parameter for chardev: > >> > >> -chardev null,id=channel0 > >> > >> and the full spice related parameters are: > >> > >> -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x8 -chardev > >> null,id=channel0 -device > >> > virtserialport,bus=virtio-serial0.0,nr=0,chardev=channel0,name=com.redhat.spice.0 > >> -usb -device usb-tablet,id=input0 -spice > >> port=5900,addr=127.0.0.1,disable-ticketing -vga qxl -device > >> qxl,id=video1,bus=pci.0,addr=0x7 > >> > >> while spice people adviced: > >> > >> -chardev spicevmc,id=channel0,name=vdagent > >> > >> and the rest of the parameters to match it. See below mail on the > details. > >> I don't know if this is really the issue, but I also recognize the > following > >> inside the domain XML: > >> > >> > >> > >> > >> > >> > >> the "channel type" is listed as "null", while I assume it should have > been > >> listed as "spicevmc". (not sure of this, I saw this in some other > >> websites). When I edit the domain xml with virsh edit, it saves my > changes > >> but the "null" stays the same how many times I try to change it. > >> > >> Please note that I've applied the following patches to virtinst 0.500.6: > >> > >> constrain-spicevmc-usage-correct.patch > >> virtinst-fix-channel-parse.patch > >> virtinst-spicevmc-fixes.patch > >> > >> which I obtained from the git. I also patched virt-manager 0.8.7 with > the > >> following I obtained from the git: > >> > >> chardev-hide-unsupported-params-for-selected-type.patch > >> only-show-relevant-char-device-fields.patch > >> show-char-device-target-name.patch > >> chardev-propose-to-add-remove-spice-agent.patch > >> allow-setting-char-device-target-name.patch > >> fix-adding-removing-channel-device.patch > >> > >> Any idea what I might be missing to get the vdagent run inside the > windows > >> guest? > >> > >> Many thanks, > >> > >> Emre Erenoglu > > What libvirt version are you using? spicevmc requires libvirt 0.8.8. > However > if libvirt is silently reverting to 'null' it's a bug either way. > > Hi Cole, I recognized that I was using 0.8.7 since our iptables version does not support the newly added "CHECKSUM" parameter that libvirt used for iptables commands. I disabled this by patching it and now libvirt 0.9.0 runs good, with the vdagent in the guest also working OK. Many thanks for all who spent their time to fix this issue. yes, libvirt 0.8.7 was silently reverting the change I made manually in the xml file. Neither virsh edit nor editing the xml and re-defining it worked. Once libvirt saw the "spicevmc" there, it just removed it and put "null" instead. -- Emre -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make sure DNSMASQ_STATE_DIR exists
On Mon, Apr 25, 2011 at 03:05:51AM -0400, Laine Stump wrote:
> On 04/24/2011 04:02 AM, Guido Günther wrote:
> >Hi,
> >
> >otherwise the directory returned by networkDnsmasqLeaseFileName will not
> >be created if ipdef->nhosts == 0 in networkBuildDnsmasqArgv.
> >
> >O.k. to apply?
> >
> >Cheers,
> > -- Guido
> >
> >---
> > src/network/bridge_driver.c |7 +++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> >
> >diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> >index 8b5c1b6..ed78710 100644
> >--- a/src/network/bridge_driver.c
> >+++ b/src/network/bridge_driver.c
> >@@ -662,6 +662,13 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
> > goto cleanup;
> > }
> >
> >+if ((err = virFileMakePath(DNSMASQ_STATE_DIR)) != 0) {
> >+virReportSystemError(err,
> >+ _("cannot create directory %s"),
> >+ DNSMASQ_STATE_DIR);
> >+goto cleanup;
> >+}
> >+
> > cmd = virCommandNew(DNSMASQ);
> > if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd)< 0) {
> > goto cleanup;
>
> ACK.
Pushed. Thanks,
-- Guido
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Lower stack usage below 4096 bytes
On 04/25/2011 10:57 AM, Eric Blake wrote:
> On 04/24/2011 04:26 PM, Matthias Bolte wrote:
>> Make virtTestLoadFile allocate the buffer to read the file into.
>>
>> Fix logic error in virtTestLoadFile, stop reading on the an empty line.
>>
>> Use virFileReadLimFD in virtTestCaptureProgramOutput.
>> ---
>> +++ b/tests/commandhelper.c
>> @@ -99,8 +99,8 @@ int main(int argc, char **argv) {
>> }
>>
>> fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no");
>> -char cwd[1024];
>> -if (!getcwd(cwd, sizeof(cwd)))
>> +char *cwd = NULL;
>> +if (!(cwd = getcwd(NULL, 0)))
>
> Ouch. This is not portable to POSIX, and while gnulib can guarantee
> that it works, the current gnulib getcwd module is GPL (and relies on
> openat, which is a rather heavy-weight replacement!).
I realize my statement may have come across as rather cryptic, so here's
some more details:
POSIX states that getcwd(NULL, n) has unspecified behavior. On most
modern systems, it malloc's n bytes, or if n is 0, the perfect size for
the answer; but older Solaris would fail with EINVAL, and it is possible
that there were other old OS that failed with a core dump.
POSIX also states that getcwd(buf, n) must fail with ERANGE if buf was
too small, so that you can manage the malloc()s yourself and iteratively
try larger buffers until you succeed (or, less likely, fail for some
other reason like EACCES); at least tools/virsh.c was already using this
idiom.
Meanwhile, modern Linux mishandles getcwd() for large directories. The
kernel refuses to return the current working directory if it is larger
than a page (only possible for super-deep hierarchies, but Jim Meyering
is fond of creating those as stress-tests for coreutils and gnulib).
/proc/self/cwd might fare better, but is probably another instance of
the kernel being likely to give up if the absolute name gets too long.
glibc has fallbacks in place for when the syscall fails, which involve
readdir() over progressively longer ../../ chains to learn the name of
each parent directory, and by using openat() it is possible to still do
this in linear instead of O(n^2) time without having to use chdir().
However, these fallbacks can fail if any directory in the middle has
permissions issues, such as no search permissions.
The end result: portable code must _always_ be prepared for getcwd() to
fail (and not just due to ENOMEM). And while it is always possible to
manage malloc() yourself, that gets to be tedious, so it would be nice
to make gnulib guarantee that getcwd(NULL,0) manages malloc().
The current gnulib module for getcwd is the least likely to fail - it
takes the best of all worlds (syscall, /proc/self/cwd, and openat()
../../ traversal) into some pretty complex code that has very few
failure cases (it can succeed in some places where glibc does not);
however, that complexity comes with some pretty heavy weight (the gnulib
openat() module can cause a call to exit(), so it is not library-safe,
and is therefore only GPL code and can't be used in LGPL libvirt).
So I'm working on adding a new gnulib module getcwd-lgpl, which
guarantees the allocation for getcwd(NULL,0), but doesn't address the
possible problems with super-deep hierarchies. That is, the version is
more likely to fail than the robust version used in GNU coreutils' pwd,
but those failures are in corner cases unlikely to happen (no one
reinstalls libvirtd into a super-deep directory) or where we can safely
pass failure back to the caller (it's now up to the user to bypass their
super-deep hierarchy in some other manner, since libvirt won't do it),
which seems like a reasonable trade-off.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] Fix small memory leaks in config parsing related functions
2011/4/25 Eric Blake : > On 04/24/2011 04:26 PM, Matthias Bolte wrote: >> Found by 'make -C tests valgrind'. >> >> xen_xm.c: Dummy allocation via virDomainChrDefNew is directly >> overwritten and lost. Free 'script' in success path too. >> >> vmx.c: Free virtualDev_string in success path too. >> >> domain_conf.c: Free compression in success path too. >> --- >> src/conf/domain_conf.c | 1 + >> src/vmx/vmx.c | 16 +--- >> src/xenxs/xen_xm.c | 3 +-- >> 3 files changed, 11 insertions(+), 9 deletions(-) > > ACK to all three fixes. > Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add virDomainEventRebootNew
2011/4/25 Eric Blake :
> On 04/25/2011 05:36 AM, Matthias Bolte wrote:
>> This will be used in the ESX driver event handling.
>> ---
>> src/conf/domain_event.c | 8
>> src/conf/domain_event.h | 1 +
>> 2 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
>> index f0380d3..688bf6c 100644
>> --- a/src/conf/domain_event.c
>> +++ b/src/conf/domain_event.c
>> @@ -572,11 +572,19 @@ virDomainEventPtr
>> virDomainEventNewFromDef(virDomainDefPtr def, int type, int de
>> return virDomainEventNew(def->id, def->name, def->uuid, type, detail);
>> }
>>
>> +virDomainEventPtr virDomainEventRebootNew(int id, const char *name,
>> + const unsigned char *uuid)
>> +{
>> + return virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_REBOOT,
>> + id, name, uuid);
>
> ACK.
>
> Do you also need to add it to src/libvirt_private.syms?
>
Yes, I missed that. I add it and pushed the result.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Lower stack usage below 4096 bytes
2011/4/25 Matthias Bolte :
> 2011/4/25 Eric Blake :
>> On 04/24/2011 04:26 PM, Matthias Bolte wrote:
>>> Make virtTestLoadFile allocate the buffer to read the file into.
>>>
>>> Fix logic error in virtTestLoadFile, stop reading on the an empty line.
>>>
>>> Use virFileReadLimFD in virtTestCaptureProgramOutput.
>>> ---
>>> +++ b/tests/commandhelper.c
>>> @@ -99,8 +99,8 @@ int main(int argc, char **argv) {
>>> }
>>>
>>> fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no");
>>> - char cwd[1024];
>>> - if (!getcwd(cwd, sizeof(cwd)))
>>> + char *cwd = NULL;
>>> + if (!(cwd = getcwd(NULL, 0)))
>>
>> Ouch. This is not portable to POSIX, and while gnulib can guarantee
>> that it works, the current gnulib getcwd module is GPL (and relies on
>> openat, which is a rather heavy-weight replacement!).
>>
>> I'm going to work on a gnulib module getcwd-lgpl which doesn't fix all
>> the known bugs in getcwd, but at least guarantees that getcwd(NULL,0)
>> will malloc insofar as the underlying getcwd is not buggy; we'll need to
>> import that into libvirt before applying the rest of this patch.
>>
>> I haven't closely reviewed the rest of this patch yet, but like the
>> general idea once we have getcwd sorted out.
>>
>
> Oops. At first I used getcwd(NULL, 0) to replace the getcwd(cwd,
> sizeof(cwd)) calls in the main functions, but then decided to just
> move the cwd buffer to global scope instead. I just missed this one in
> the second rewrite round.
>
> Matthias
>
Grep'ing the rest of the codebase shows a getcwd(NULL, 0) call in
virFileAbsPath that probably needs care too.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Lower stack usage below 4096 bytes
2011/4/25 Eric Blake :
> On 04/24/2011 04:26 PM, Matthias Bolte wrote:
>> Make virtTestLoadFile allocate the buffer to read the file into.
>>
>> Fix logic error in virtTestLoadFile, stop reading on the an empty line.
>>
>> Use virFileReadLimFD in virtTestCaptureProgramOutput.
>> ---
>> +++ b/tests/commandhelper.c
>> @@ -99,8 +99,8 @@ int main(int argc, char **argv) {
>> }
>>
>> fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no");
>> - char cwd[1024];
>> - if (!getcwd(cwd, sizeof(cwd)))
>> + char *cwd = NULL;
>> + if (!(cwd = getcwd(NULL, 0)))
>
> Ouch. This is not portable to POSIX, and while gnulib can guarantee
> that it works, the current gnulib getcwd module is GPL (and relies on
> openat, which is a rather heavy-weight replacement!).
>
> I'm going to work on a gnulib module getcwd-lgpl which doesn't fix all
> the known bugs in getcwd, but at least guarantees that getcwd(NULL,0)
> will malloc insofar as the underlying getcwd is not buggy; we'll need to
> import that into libvirt before applying the rest of this patch.
>
> I haven't closely reviewed the rest of this patch yet, but like the
> general idea once we have getcwd sorted out.
>
Oops. At first I used getcwd(NULL, 0) to replace the getcwd(cwd,
sizeof(cwd)) calls in the main functions, but then decided to just
move the cwd buffer to global scope instead. I just missed this one in
the second rewrite round.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Lower stack usage below 4096 bytes
On 04/24/2011 04:26 PM, Matthias Bolte wrote:
> Make virtTestLoadFile allocate the buffer to read the file into.
>
> Fix logic error in virtTestLoadFile, stop reading on the an empty line.
>
> Use virFileReadLimFD in virtTestCaptureProgramOutput.
> ---
> +++ b/tests/commandhelper.c
> @@ -99,8 +99,8 @@ int main(int argc, char **argv) {
> }
>
> fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no");
> -char cwd[1024];
> -if (!getcwd(cwd, sizeof(cwd)))
> +char *cwd = NULL;
> +if (!(cwd = getcwd(NULL, 0)))
Ouch. This is not portable to POSIX, and while gnulib can guarantee
that it works, the current gnulib getcwd module is GPL (and relies on
openat, which is a rather heavy-weight replacement!).
I'm going to work on a gnulib module getcwd-lgpl which doesn't fix all
the known bugs in getcwd, but at least guarantees that getcwd(NULL,0)
will malloc insofar as the underlying getcwd is not buggy; we'll need to
import that into libvirt before applying the rest of this patch.
I haven't closely reviewed the rest of this patch yet, but like the
general idea once we have getcwd sorted out.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] Fix small memory leaks in config parsing related functions
On 04/24/2011 04:26 PM, Matthias Bolte wrote: > Found by 'make -C tests valgrind'. > > xen_xm.c: Dummy allocation via virDomainChrDefNew is directly > overwritten and lost. Free 'script' in success path too. > > vmx.c: Free virtualDev_string in success path too. > > domain_conf.c: Free compression in success path too. > --- > src/conf/domain_conf.c |1 + > src/vmx/vmx.c | 16 +--- > src/xenxs/xen_xm.c |3 +-- > 3 files changed, 11 insertions(+), 9 deletions(-) ACK to all three fixes. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] Add virDomainEventRebootNew
On 04/25/2011 05:36 AM, Matthias Bolte wrote:
> This will be used in the ESX driver event handling.
> ---
> src/conf/domain_event.c |8
> src/conf/domain_event.h |1 +
> 2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> index f0380d3..688bf6c 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -572,11 +572,19 @@ virDomainEventPtr
> virDomainEventNewFromDef(virDomainDefPtr def, int type, int de
> return virDomainEventNew(def->id, def->name, def->uuid, type, detail);
> }
>
> +virDomainEventPtr virDomainEventRebootNew(int id, const char *name,
> + const unsigned char *uuid)
> +{
> +return virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_REBOOT,
> + id, name, uuid);
ACK.
Do you also need to add it to src/libvirt_private.syms?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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] util: warn when passing a non-pointer to VIR_FREE
On 04/24/2011 04:17 AM, Matthias Bolte wrote: >> So how about: >> >> diff --git i/src/util/memory.h w/src/util/memory.h >> index 66b4c42..d77a295 100644 >> --- i/src/util/memory.h >> +++ w/src/util/memory.h >> @@ -1,7 +1,7 @@ >> /* >> * memory.c: safer memory allocation >> * >> - * Copyright (C) 2010 Red Hat, Inc. >> + * Copyright (C) 2010-2011 Red Hat, Inc. >> * Copyright (C) 2008 Daniel P. Berrange >> * >> * This library is free software; you can redistribute it and/or >> @@ -197,7 +197,11 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); >> * Free the memory stored in 'ptr' and update to point >> * to NULL. >> */ >> -# define VIR_FREE(ptr) virFree(&(ptr)) >> +/* The ternary ensures that ptr is a pointer and not an integer type, >> + * while evaluating ptr only once. For now, we intentionally cast >> + * away const, since a number of callers safely pass const char *. >> + */ >> +# define VIR_FREE(ptr) virFree((void *) (1 ? (const void *) &(ptr) : >> (ptr))) >> >> >> # if TEST_OOM >> > > ACK, to your improved version. Pushed with this commit message: commit 90d761eeb26c9619571b68a8863b8425a33555d6 Author: Eric Blake Date: Fri Apr 22 20:15:50 2011 -0600 build: make VIR_FREE do some type checking We can exploit the fact that gcc warns about int-to-pointer conversion in ternary cond?(void*):(int) in order to prevent future mistakes of calling VIR_FREE on a scalar lvalue. For example, between commits 158ba873 and 802e2df, we would have had this warning: cc1: warnings being treated as errors remote.c: In function 'remoteDispatchListNetworks': remote.c:3684:70: error: pointer/integer type mismatch in conditional expression There are still a number of places that malloc into a const char*; while it would probably be worth scrubbing them to use char* instead, that is a separate patch, so we have to cast away const in VIR_FREE for now. * src/util/memory.h (VIR_FREE): Make gcc warn about integers. Iteratively developed from a patch by Christophe Fergeau. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] threads: add one-time initialization support
On 04/24/2011 04:14 AM, Matthias Bolte wrote:
>
> Compared to the gnulib lock module version, this one is less complex
> and I don't get why gnulib uses 3 states for the init and complete
> variables and an additional lock object. The only reason could be to
> minimize the time busy waiting in the while loop.
It may also be that the gnulib module was trying to worry about
cancellation points, but libvirt isn't (yet) using cancellation points.
That does indeed require more complexity to handle an init callback
that gets cancelled, where another thread then has to take over the
one-shot initiailization.
>
>> +
>> +struct virOnceControl {
>> +long init; /* 0 at startup, > 0 if init has started */
>> +volatile long complete; /* 0 until first thread completes callback */
>> +};
>
> MSDN docs about InterlockedIncrement suggest that init should be volatile too.
>
> ACK, with init marked volatile.
Thanks, and pushed with that modification. I guess that also means that
the virObject patches should also be marking its reference-counter
variable as volatile.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Unlink managed state file when a domain is undefined
On 04/25/2011 08:29 AM, Eric Blake wrote: > On 04/25/2011 03:41 AM, Osier Yang wrote: >> The managed state file is not useful anymore after the domain is >> undefined, and perhaps cause confusion. E.g. define & start a domain >> which has same name but different UUID with previous undefined >> domain later. >> >> v1 - v2: >>* Try to delete the managed state file before delete domain >> config file, and goto fail if it failed to delete it. >> --- >> src/qemu/qemu_driver.c | 10 ++ >> 1 files changed, 10 insertions(+), 0 deletions(-) > > Ouch. virDomainUndefine doesn't have a flags argument. But this is > changing behavior in a user-visible manner (arguably for the better, but > any change is risky) > > We have two options: > > 1. Proceed with the change: virDomainUndefine removes all associated > snapshot data; but this is a silent change in behavior > > 2. Add a new API: virDomainUndefineFlags; flag 0 leaves snapshot data > around (but warns), flag VIR_DOMAIN_UNDEFINE_ALL_ASSOCIATED_DATA (or > some better name) guarantees that snapshots are also removed Option 3: no silent deletion on undefine, and no new API, but make virDomainUndefine fail if the managed state file exists. The user is then responsible for using virDomainHasManagedSaveImage and virDomainManagedSaveRemove prior to undefining a domain. But whereas with option 1, people might complain that we removed the managed state file, now with option 3 people might complain that we prevent the undefine of a domain. Which leads to: Option 4: Add virDomainUndefineFlags(). virDomainUndefineFlags(,0) refuses to undefine a domain if state exists, while virDomainUndefineFlags(,VIR_DOMAIN_UNDEFINE_FORCE) proceeds to undefine the domain even if a managed state file exists. We'd still have to decide whether virDomainUndefine maps to Flags(,0) or Flags(,FORCE), but at least the default option is safer than in option 2 where you have to use a flag to get the safe behavior. At any rate, at the virsh level we can add a flag that maps into two API calls as necessary to use virDomainManagedSaveRemove prior to virDomainUndefine, regardless of what we do at the API level. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Unlink managed state file when a domain is undefined
On 04/25/2011 03:41 AM, Osier Yang wrote: > The managed state file is not useful anymore after the domain is > undefined, and perhaps cause confusion. E.g. define & start a domain > which has same name but different UUID with previous undefined > domain later. > > v1 - v2: >* Try to delete the managed state file before delete domain > config file, and goto fail if it failed to delete it. > --- > src/qemu/qemu_driver.c | 10 ++ > 1 files changed, 10 insertions(+), 0 deletions(-) Ouch. virDomainUndefine doesn't have a flags argument. But this is changing behavior in a user-visible manner (arguably for the better, but any change is risky) We have two options: 1. Proceed with the change: virDomainUndefine removes all associated snapshot data; but this is a silent change in behavior 2. Add a new API: virDomainUndefineFlags; flag 0 leaves snapshot data around (but warns), flag VIR_DOMAIN_UNDEFINE_ALL_ASSOCIATED_DATA (or some better name) guarantees that snapshots are also removed Being an API question, I think danpb might need to weigh in before we make a change. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [virt-tools-list] libvirt spice command line appears to be incorrect
On 04/25/2011 04:27 AM, Richard W.M. Jones wrote: > [Copying this to libvir-list] > > On Mon, Apr 25, 2011 at 01:07:37AM +0400, Emre Erenoglu wrote: >> Hi, >> >> I'm the package maintainer for virt-manager and related packages for Pardus >> distribution. While testing the latest libvirt, virtinst & virt-manager >> packages, I've come across a strange issue and I would like to get your >> valuable opinion. >> >> I add all spice related devices and everything works good, except the >> vdagent inside the windows xp guest. The virtio serial driver is loaded >> correctly. As I track down the issue, I found out that libvirt is starting >> qemu-kvm with parameters which do not match the ones adviced by the spice >> people. Please see below email discussion with them on this. The offending >> line seems to be the chardev parameter. qemu-kvm is started by virt-manager >> with the following parameter for chardev: >> >> -chardev null,id=channel0 >> >> and the full spice related parameters are: >> >> -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x8 -chardev >> null,id=channel0 -device >> virtserialport,bus=virtio-serial0.0,nr=0,chardev=channel0,name=com.redhat.spice.0 >> -usb -device usb-tablet,id=input0 -spice >> port=5900,addr=127.0.0.1,disable-ticketing -vga qxl -device >> qxl,id=video1,bus=pci.0,addr=0x7 >> >> while spice people adviced: >> >> -chardev spicevmc,id=channel0,name=vdagent >> >> and the rest of the parameters to match it. See below mail on the details. >> I don't know if this is really the issue, but I also recognize the following >> inside the domain XML: >> >> >> >> >> >> >> the "channel type" is listed as "null", while I assume it should have been >> listed as "spicevmc". (not sure of this, I saw this in some other >> websites). When I edit the domain xml with virsh edit, it saves my changes >> but the "null" stays the same how many times I try to change it. >> >> Please note that I've applied the following patches to virtinst 0.500.6: >> >> constrain-spicevmc-usage-correct.patch >> virtinst-fix-channel-parse.patch >> virtinst-spicevmc-fixes.patch >> >> which I obtained from the git. I also patched virt-manager 0.8.7 with the >> following I obtained from the git: >> >> chardev-hide-unsupported-params-for-selected-type.patch >> only-show-relevant-char-device-fields.patch >> show-char-device-target-name.patch >> chardev-propose-to-add-remove-spice-agent.patch >> allow-setting-char-device-target-name.patch >> fix-adding-removing-channel-device.patch >> >> Any idea what I might be missing to get the vdagent run inside the windows >> guest? >> >> Many thanks, >> >> Emre Erenoglu What libvirt version are you using? spicevmc requires libvirt 0.8.8. However if libvirt is silently reverting to 'null' it's a bug either way. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] build: use gnulib passfd for simpler SCM_RIGHTS code
On 04/25/2011 01:17 AM, Laine Stump wrote:
>> @@ -1529,26 +1513,20 @@ virFileOpenAs(const char *path, int openflags,
>> mode_t mode,
>> VIR_FORCE_CLOSE(pair[1]);
>>
>> do {
>> -ret = recvmsg(pair[0],&msg, 0);
>> +ret = recvfd(pair[0], 0);
>> } while (ret< 0&& errno == EINTR);
ret == -1 and errno == EACCES on failure to transfer fd...
>>
>> -if (ret< 0) {
>> +if (ret< 0&& errno != EACCES) {
>> ret = -errno;
>> VIR_FORCE_CLOSE(pair[0]);
>> while ((waitret = waitpid(pid, NULL, 0) == -1)
>> && (errno == EINTR));
>> goto parenterror;
>> +} else {
>> +fd = ret;
>> }
so now fd == -1...
> if (!WIFEXITED(status) || (ret = -WEXITSTATUS(status)) == -EACCES ||
> fd == -1) {
> /* fall back to the simpler method, which works better in
> * some cases */
> return virFileOpenAsNoFork(path, openflags, mode, uid, gid,
> flags);
> }
so this uses the fallback code, regardless of child exit status, and we
also ensured that the child got reaped.
>
> What if errno == EACCES? Will we be getting all the error recovery we
> need in the caller?
Yes.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
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-php] Work on ./example
Hello, as part of my student project I am working on simple web interface which would support multiple virtualisation platforms. This of course led me to libvirt and libvirt-php more specificly. Right now I'm using modfied code of exapme Libvirt class and the plan is: - add phpdoc - refactor it to match libvirt convetion (get_domain_someting -> dmain_get_something) - split Libvirt to subclasses for domain, connection, network, storage (not completly sure if it is good idea) - add few unit tests (dont know about time and usefulness of this) Would it be welcomed if I send to whole think or even parts of it as patches to libvirt-php/example? Thanks for any reply and sorry for my english, David -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add virDomainEventRebootNew
This will be used in the ESX driver event handling.
---
src/conf/domain_event.c |8
src/conf/domain_event.h |1 +
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index f0380d3..688bf6c 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -572,11 +572,19 @@ virDomainEventPtr
virDomainEventNewFromDef(virDomainDefPtr def, int type, int de
return virDomainEventNew(def->id, def->name, def->uuid, type, detail);
}
+virDomainEventPtr virDomainEventRebootNew(int id, const char *name,
+ const unsigned char *uuid)
+{
+return virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_REBOOT,
+ id, name, uuid);
+}
+
virDomainEventPtr virDomainEventRebootNewFromDom(virDomainPtr dom)
{
return virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_REBOOT,
dom->id, dom->name, dom->uuid);
}
+
virDomainEventPtr virDomainEventRebootNewFromObj(virDomainObjPtr obj)
{
return virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_REBOOT,
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index e28293d..c03a159 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -112,6 +112,7 @@ virDomainEventPtr virDomainEventNewFromDom(virDomainPtr
dom, int type, int detai
virDomainEventPtr virDomainEventNewFromObj(virDomainObjPtr obj, int type, int
detail);
virDomainEventPtr virDomainEventNewFromDef(virDomainDefPtr def, int type, int
detail);
+virDomainEventPtr virDomainEventRebootNew(int id, const char *name, const
unsigned char *uuid);
virDomainEventPtr virDomainEventRebootNewFromDom(virDomainPtr dom);
virDomainEventPtr virDomainEventRebootNewFromObj(virDomainObjPtr obj);
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix dynamic dispatch for CastFromAnyType functions
2011/4/25 Matthias Bolte :
> This was broken by the refactoring in ac1e6586ec75. It resulted in a
> segfault for 'virsh vol-dumpxml' and related volume functions.
>
Actually that patch doesn't fix the problem correctly. It just turns
the segfault into an error message.
Here's v2 that really fixes the problem. Note the important difference
between anyType->_type and anyType->type :)
Matthias
From a17245fb92c88439ffbb84e34bebf1afb6903885 Mon Sep 17 00:00:00 2001
From: Matthias Bolte
Date: Mon, 25 Apr 2011 12:38:17 +0200
Subject: [PATCH] esx: Fix dynamic dispatch for CastFromAnyType functions
This was broken by the refactoring in ac1e6586ec75. It resulted in a
segfault for 'virsh vol-dumpxml' and related volume functions.
Before the refactoring all users of the ESX_VI__TEMPLATE__DISPATCH
macro dispatched on the item type, as the item is the input to all those
functions.
Commit ac1e6586ec75 made the dynamically dispatched CastFromAnyType
functions use this macro too, but this functions dispatched on the
actual type of the AnyType object. The item is the output of the
CastFromAnyType functions.
This difference was missed in the refactoring, making CastFromAnyType
functions dereferencing the item pointer that is NULL at the time of
the dispatch.
---
src/esx/esx_vi_types.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index dd83954..3c81021 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -533,8 +533,9 @@
* Macros to implement dynamic dispatched functions
*/
-#define ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, _error_return) \
-switch (item->_type) {\
+#define ESX_VI__TEMPLATE__DISPATCH(_actual_type, __type, _dispatch, \
+ _error_return) \
+switch (_actual_type) { \
_dispatch \
\
case esxVI_Type_##__type: \
@@ -543,7 +544,7 @@
default:\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
_("Call to %s for unexpected type '%s'"), __FUNCTION__, \
- esxVI_Type_ToString(item->_type)); \
+ esxVI_Type_ToString(_actual_type)); \
return _error_return; \
}
@@ -585,7 +586,8 @@
#define ESX_VI__TEMPLATE__DYNAMIC_FREE(__type, _dispatch, _body) \
ESX_VI__TEMPLATE__FREE(__type,\
- ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, /* nothing */)\
+ ESX_VI__TEMPLATE__DISPATCH(item->_type, __type, _dispatch, \
+ /* nothing */) \
_body)
@@ -618,14 +620,14 @@
#define ESX_VI__TEMPLATE__DYNAMIC_CAST_FROM_ANY_TYPE(__type, _dispatch) \
ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE_EXTRA(__type, esxVI_##__type,\
- ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), \
+ ESX_VI__TEMPLATE__DISPATCH(anyType->type, __type, _dispatch, -1), \
/* nothing */)
#define ESX_VI__TEMPLATE__DYNAMIC_SERIALIZE(__type, _dispatch, _serialize)\
ESX_VI__TEMPLATE__SERIALIZE_EXTRA(__type, \
- ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), \
+ ESX_VI__TEMPLATE__DISPATCH(item->_type, __type, _dispatch, -1), \
_serialize)
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: Initialize hooks at daemon shutdown if no hooks defined
We support to initialize the hooks at daemon reload if there is no hooks script is defined, should we also support initialize the hooks at daemon shutdown if no hooks is defined? To address bz: https://bugzilla.redhat.com/show_bug.cgi?id=688859 --- src/util/hooks.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/util/hooks.c b/src/util/hooks.c index a409d77..30e20ac 100644 --- a/src/util/hooks.c +++ b/src/util/hooks.c @@ -209,7 +209,8 @@ virHookCall(int driver, const char *id, int op, int sub_op, const char *extra, */ if ((virHooksFound == -1) || ((driver == VIR_HOOK_DRIVER_DAEMON) && - (op == VIR_HOOK_DAEMON_OP_RELOAD))) + (op == VIR_HOOK_DAEMON_OP_RELOAD || + op == VIR_HOOK_DAEMON_OP_SHUTDOWN))) virHookInitialize(); if ((virHooksFound & (1 << driver)) == 0) -- 1.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: Unlink managed state file when a domain is undefined
The managed state file is not useful anymore after the domain is
undefined, and perhaps cause confusion. E.g. define & start a domain
which has same name but different UUID with previous undefined
domain later.
v1 - v2:
* Try to delete the managed state file before delete domain
config file, and goto fail if it failed to delete it.
---
src/qemu/qemu_driver.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 771678e..4c5edd4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3738,6 +3738,7 @@ static int qemudDomainUndefine(virDomainPtr dom) {
struct qemud_driver *driver = dom->conn->privateData;
virDomainObjPtr vm;
virDomainEventPtr event = NULL;
+char *managed_save = NULL;
int ret = -1;
qemuDriverLock(driver);
@@ -3763,6 +3764,15 @@ static int qemudDomainUndefine(virDomainPtr dom) {
goto cleanup;
}
+if (!(managed_save = qemuDomainManagedSavePath(driver, vm)))
+goto cleanup;
+
+if (virFileExists(managed_save) && (unlink(managed_save) < 0)) {
+virReportSystemError(errno, "%s",
+ _("Failed to delete managed state file"));
+goto cleanup;
+ }
+
if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) < 0)
goto cleanup;
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fix dynamic dispatch for CastFromAnyType functions
This was broken by the refactoring in ac1e6586ec75. It resulted in a
segfault for 'virsh vol-dumpxml' and related volume functions.
Before the refactoring all users of the ESX_VI__TEMPLATE__DISPATCH
macro dispatched on the item type, as the item is the input to all those
functions.
Commit ac1e6586ec75 made the dynamically dispatched CastFromAnyType
functions use this macro too, but this functions dispatched on the
actual type of the AnyType object. The item is the output of the
CastFromAnyType functions.
This difference was missed in the refactoring, making CastFromAnyType
functions dereferencing the item pointer that is NULL at the time of
the dispatch.
---
src/esx/esx_vi_types.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index dd83954..6a2d5cf 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -533,8 +533,8 @@
* Macros to implement dynamic dispatched functions
*/
-#define ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, _error_return) \
-switch (item->_type) {\
+#define ESX_VI__TEMPLATE__DISPATCH(_object, __type, _dispatch, _error_return) \
+switch ((_object)->_type) { \
_dispatch \
\
case esxVI_Type_##__type: \
@@ -543,7 +543,7 @@
default:\
ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
_("Call to %s for unexpected type '%s'"), __FUNCTION__, \
- esxVI_Type_ToString(item->_type)); \
+ esxVI_Type_ToString((_object)->_type)); \
return _error_return; \
}
@@ -585,7 +585,7 @@
#define ESX_VI__TEMPLATE__DYNAMIC_FREE(__type, _dispatch, _body) \
ESX_VI__TEMPLATE__FREE(__type,\
- ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, /* nothing */)\
+ ESX_VI__TEMPLATE__DISPATCH(item, __type, _dispatch, /* nothing */) \
_body)
@@ -618,14 +618,14 @@
#define ESX_VI__TEMPLATE__DYNAMIC_CAST_FROM_ANY_TYPE(__type, _dispatch) \
ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE_EXTRA(__type, esxVI_##__type,\
- ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), \
+ ESX_VI__TEMPLATE__DISPATCH(anyType, __type, _dispatch, -1), \
/* nothing */)
#define ESX_VI__TEMPLATE__DYNAMIC_SERIALIZE(__type, _dispatch, _serialize)\
ESX_VI__TEMPLATE__SERIALIZE_EXTRA(__type, \
- ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), \
+ ESX_VI__TEMPLATE__DISPATCH(item, __type, _dispatch, -1),\
_serialize)
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt spice command line appears to be incorrect (was: Re: [virt-tools-list] vdagent does not start with domain defined by virt-manager 0.8.7)
[Copying this to libvir-list] On Mon, Apr 25, 2011 at 01:07:37AM +0400, Emre Erenoglu wrote: > Hi, > > I'm the package maintainer for virt-manager and related packages for Pardus > distribution. While testing the latest libvirt, virtinst & virt-manager > packages, I've come across a strange issue and I would like to get your > valuable opinion. > > I add all spice related devices and everything works good, except the > vdagent inside the windows xp guest. The virtio serial driver is loaded > correctly. As I track down the issue, I found out that libvirt is starting > qemu-kvm with parameters which do not match the ones adviced by the spice > people. Please see below email discussion with them on this. The offending > line seems to be the chardev parameter. qemu-kvm is started by virt-manager > with the following parameter for chardev: > > -chardev null,id=channel0 > > and the full spice related parameters are: > > -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x8 -chardev > null,id=channel0 -device > virtserialport,bus=virtio-serial0.0,nr=0,chardev=channel0,name=com.redhat.spice.0 > -usb -device usb-tablet,id=input0 -spice > port=5900,addr=127.0.0.1,disable-ticketing -vga qxl -device > qxl,id=video1,bus=pci.0,addr=0x7 > > while spice people adviced: > > -chardev spicevmc,id=channel0,name=vdagent > > and the rest of the parameters to match it. See below mail on the details. > I don't know if this is really the issue, but I also recognize the following > inside the domain XML: > > > > > > > the "channel type" is listed as "null", while I assume it should have been > listed as "spicevmc". (not sure of this, I saw this in some other > websites). When I edit the domain xml with virsh edit, it saves my changes > but the "null" stays the same how many times I try to change it. > > Please note that I've applied the following patches to virtinst 0.500.6: > > constrain-spicevmc-usage-correct.patch > virtinst-fix-channel-parse.patch > virtinst-spicevmc-fixes.patch > > which I obtained from the git. I also patched virt-manager 0.8.7 with the > following I obtained from the git: > > chardev-hide-unsupported-params-for-selected-type.patch > only-show-relevant-char-device-fields.patch > show-char-device-target-name.patch > chardev-propose-to-add-remove-spice-agent.patch > allow-setting-char-device-target-name.patch > fix-adding-removing-channel-device.patch > > Any idea what I might be missing to get the vdagent run inside the windows > guest? > > Many thanks, > > Emre Erenoglu > > -- Forwarded message -- > From: Marian Krcmarik > Date: Mon, Apr 18, 2011 at 5:56 PM > Subject: Re: [Spice-devel] vdagent does not start > To: Emre Erenoglu > Cc: spice-de...@lists.freedesktop.org > > > > > - Original Message - > > From: "Emre Erenoglu" > > To: spice-de...@lists.freedesktop.org > > Sent: Sunday, April 17, 2011 1:10:16 PM > > Subject: [Spice-devel] vdagent does not start > > Dear Developers, > > > > I have a virtual XP system with the spice channel enabled through the > > serial port. The command line that runs qemu has (reduced): > > > > -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x8 > > -chardev null,id=channel0 -device > > > virtserialport,bus=virtio-serial0.0,nr=0,chardev=channel0,name=com.redhat.spice.0 > > -usb -device usb-tablet,id=input0 -spice > > port=5900,addr=127.0.0.1,disable-ticketing -vga qxl -device > > qxl,id=video1,bus=pci.0,addr=0x7 > > I think you may need to specify chardev for spice so I would modify: > > -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x8 -chardev > spicevmc,id=channel0,name=vdagent -device > virtserialport,bus=virtio-serial0.0,nr=0,chardev=channel0,name=com.redhat.spice.0 > -usb -device usb-tablet,id=input0 -spice > port=5900,addr=127.0.0.1,disable-ticketing -vga qxl -device > qxl,id=video1,bus=pci.0,addr=0x7 > > with agent and virtio-serial driver installed on guest. > > > > However, the vdagent services does not start. when I give it a start > > control, it reports to start then stop immediately. Here are the logs > > I've found: > > > > -- > Emre -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix 32-bit test failure
On 04/21/2011 10:26 AM, Eric Blake wrote:
ARRAY_CARDINALITY is typed as size_t, not long; this matters on 32-bit
platforms:
hashtest.c: In function 'testHashRemoveForEach':
hashtest.c:114: error: format '%lu' expects type 'long unsigned int', but
argument 4 has type 'unsigned int' [-Wformat]
* tests/hashtest.c (testHashRemoveForEach): Use correct format.
---
Pushing under the build-breaker rule.
tests/hashtest.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tests/hashtest.c b/tests/hashtest.c
index dff0181..722b44c 100644
--- a/tests/hashtest.c
+++ b/tests/hashtest.c
@@ -112,7 +112,7 @@ testHashRemoveForEach(const void *data)
if (count != ARRAY_CARDINALITY(uuids)) {
if (virTestGetVerbose()) {
testError("\nvirHashForEach didn't go through all entries,"
- " %d != %lu\n",
+ " %d != %zu\n",
count, ARRAY_CARDINALITY(uuids));
}
goto cleanup;
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] build: use gnulib passfd for simpler SCM_RIGHTS code
On 04/21/2011 05:17 PM, Eric Blake wrote:
* .gnulib: Update to latest for passfd fixes.
* bootstrap.conf (gnulib_modules): Add passfd.
* src/util/util.c (virFileOpenAs): Simplify.
---
Now that the mingw side of passfd is fixed in gnulib, I can
resubmit this patch.
v2: update .gnulib, no other changes
.gnulib |2 +-
bootstrap.conf |1 +
src/util/util.c | 38 --
3 files changed, 10 insertions(+), 31 deletions(-)
diff --git a/.gnulib b/.gnulib
index fb79969..5a9e46a 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit fb799692f5bb43310424977e0ca15599fc68d776
+Subproject commit 5a9e46ab46042f007426c1e06b836cf5608d8d4a
diff --git a/bootstrap.conf b/bootstrap.conf
index 293f86e..3b3a90f 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -52,6 +52,7 @@ mkstemps
mktempd
netdb
nonblocking
+passfd
perror
physmem
pipe-posix
diff --git a/src/util/util.c b/src/util/util.c
index d4d2610..de4e3b3 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -78,6 +78,7 @@
#include "files.h"
#include "command.h"
#include "nonblocking.h"
+#include "passfd.h"
#ifndef NSIG
# define NSIG 32
@@ -1480,11 +1481,6 @@ virFileOpenAs(const char *path, int openflags, mode_t
mode,
int waitret, status, ret = 0;
int fd = -1;
int pair[2] = { -1, -1 };
-struct msghdr msg;
-struct cmsghdr *cmsg;
-char buf[CMSG_SPACE(sizeof(fd))];
-char dummy = 0;
-struct iovec iov;
int forkRet;
if ((!(flags& VIR_FILE_OPEN_AS_UID))
@@ -1506,18 +1502,6 @@ virFileOpenAs(const char *path, int openflags, mode_t
mode,
return ret;
}
-memset(&msg, 0, sizeof(msg));
-iov.iov_base =&dummy;
-iov.iov_len = 1;
-msg.msg_iov =&iov;
-msg.msg_iovlen = 1;
-msg.msg_control = buf;
-msg.msg_controllen = sizeof(buf);
-cmsg = CMSG_FIRSTHDR(&msg);
-cmsg->cmsg_level = SOL_SOCKET;
-cmsg->cmsg_type = SCM_RIGHTS;
-cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
-
forkRet = virFork(&pid);
if (pid< 0) {
@@ -1529,26 +1513,20 @@ virFileOpenAs(const char *path, int openflags, mode_t
mode,
VIR_FORCE_CLOSE(pair[1]);
do {
-ret = recvmsg(pair[0],&msg, 0);
+ret = recvfd(pair[0], 0);
} while (ret< 0&& errno == EINTR);
-if (ret< 0) {
+if (ret< 0&& errno != EACCES) {
ret = -errno;
VIR_FORCE_CLOSE(pair[0]);
while ((waitret = waitpid(pid, NULL, 0) == -1)
&& (errno == EINTR));
goto parenterror;
+} else {
+fd = ret;
}
What if errno == EACCES? Will we be getting all the error recovery we
need in the caller?
VIR_FORCE_CLOSE(pair[0]);
-/* See if fd was transferred. */
-cmsg = CMSG_FIRSTHDR(&msg);
-if (cmsg&& cmsg->cmsg_len == CMSG_LEN(sizeof(fd))&&
-cmsg->cmsg_level == SOL_SOCKET&&
-cmsg->cmsg_type == SCM_RIGHTS) {
-memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd));
-}
-
/* wait for child to complete, and retrieve its exit code */
while ((waitret = waitpid(pid,&status, 0) == -1)
&& (errno == EINTR));
@@ -1557,12 +1535,14 @@ virFileOpenAs(const char *path, int openflags, mode_t
mode,
virReportSystemError(errno,
_("failed to wait for child creating '%s'"),
path);
+VIR_FORCE_CLOSE(fd);
goto parenterror;
}
if (!WIFEXITED(status) || (ret = -WEXITSTATUS(status)) == -EACCES ||
fd == -1) {
/* fall back to the simpler method, which works better in
* some cases */
+VIR_FORCE_CLOSE(fd);
return virFileOpenAsNoFork(path, openflags, mode, uid, gid,
flags);
}
if (!ret)
@@ -1627,10 +1607,9 @@ parenterror:
path, mode);
goto childerror;
}
-memcpy(CMSG_DATA(cmsg),&fd, sizeof(fd));
do {
-ret = sendmsg(pair[1],&msg, 0);
+ret = sendfd(pair[1], fd);
} while (ret< 0&& errno == EINTR);
if (ret< 0) {
@@ -1638,7 +1617,6 @@ parenterror:
goto childerror;
}
-ret = 0;
childerror:
/* ret tracks -errno on failure, but exit value must be positive.
* If the child exits with EACCES, then the parent tries again. */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make sure DNSMASQ_STATE_DIR exists
On 04/24/2011 04:02 AM, Guido Günther wrote:
Hi,
otherwise the directory returned by networkDnsmasqLeaseFileName will not
be created if ipdef->nhosts == 0 in networkBuildDnsmasqArgv.
O.k. to apply?
Cheers,
-- Guido
---
src/network/bridge_driver.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8b5c1b6..ed78710 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -662,6 +662,13 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
goto cleanup;
}
+if ((err = virFileMakePath(DNSMASQ_STATE_DIR)) != 0) {
+virReportSystemError(err,
+ _("cannot create directory %s"),
+ DNSMASQ_STATE_DIR);
+goto cleanup;
+}
+
cmd = virCommandNew(DNSMASQ);
if (networkBuildDnsmasqArgv(network, ipdef, pidfile, cmd)< 0) {
goto cleanup;
ACK.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
