[libvirt] QEMU has applied for Google Summer of Code 2013
QEMU.org has applied for Google Summer of Code 2013 and also aims to be an umbrella organization for libvirt and the KVM kernel module. Accepted mentoring organizations will be announced on April 8 at 19:00 UTC at http://google-melange.com/. This year we have proposed 5 QEMU project ideas, 1 KVM kernel module project idea, and 4 libvirt project ideas: http://qemu-project.org/Google_Summer_of_Code_2013 Thanks to everyone who has volunteered to be a mentor! Also thanks to Anthony Liguori for being backup org admin. Fingers crossed, Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make virsh support '~' and '$HOME' in interactive mode
On Thu, Mar 21, 2013 at 09:43:02PM -0600, Eric Blake wrote: Expanding everything means re-implementing what the shell does. wordexp() would be ideal for this, except that wordexp() is not portable enough. Maybe wordexp() could be used on platform which support it, and on platforms that do not have it, it would not be used, and env var would not be expanded? Or do we want to have identical behaviour for virsh on all platforms to avoid confusion? Christophe pgpYZlfTGRBhh.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] doc: write separate module for hostdev passthrough and in-use tracking
Hi, List,
As the mail I've sent a week before:
https://www.redhat.com/archives/libvir-list/2013-March/msg00730.html
I'm willing to push this work forward so that the passthrough APIs
could be reused by qemu driver and libxl driver (which doesn't support
pci passthrough yet and tries to add this function recently), or other
drivers.
But since this work affacts a lot, I'm not sure if I can control it in
a correct way. I write a draft to describe what I'm considering how to
do, as in the following and in attachment. Hope to get your review,
comment and guidence to improve the work before start coding. Any
feedback will be very appreciated!
Thanks!
Chunyan
DRAFT:
Write separate module for hostdev passthrough
1. Purposes:
* Move hostdev passthrough APIs from qemu_hostdev.ch to separate module so
that they could be reused by other hypervisors too
* Maintain global in-use state of hostdevs
2. Module design (draft):
* New module name: hostdev_driver
* New module files: hostdev_driver.ch hostdev_conf.ch
* New Definitions:
## [src/driver.h]
typedef struct _virHostdevDriver virHostdevDriver;
typedef virHostdevDriver *virHostdevDriverPtr;
struct _virHosedevDriver {
const char *name;
virDrvOpen open;
virDrvClose close;
virDrvPrepareHostdevsprepareHostdevs;
virDrvPreparePciHostdevspreparePciHostdevs;
virDrvprepareUsbHostdevsprepareUsbHostdevs;
virDrvReattachHostdevsreattachHostdevs;
virDrvReattachPciHostdevsreattachPciHostdevs;
virDrvReattachUsbHostdevs reattachUsbHostdevs;
virDrvGetActivePciHostdevList getActivePciHostdevList;
virDrvGetActiveUsbHostdevList getActiveUsbHostdevList;
virDrvGetDomainActivePciHostdevList
getDomainActivePciHostdevList;
virDrvGetDomainActiveUsbHostdevList
getDomainActiveUsbHostdevList;
};
## [src/hostdev/hostdev_conf.h]
typedef struct _virHostdevDriverState virHostdevDriverState;
typedef virHostdevDriverState *virHostdevDriverStatePtr;
struct _virHostdevDriverState {
virMutex lock;
virPCIDeviceListPtr activePciHostdevs;
virPCIDeviceListPtr inactivePciHostdevs;
virUSBDeviceListPtr activeUsbHostdevs;
};
## [src/hostdev/hostdev_driver.c]
static virHostdevDriver hostdevDriver = {
.name = hostdev,
.open = hostdevDriverOpen,
.close = hostdevDriverClose,
.prepareHostdevs = virPrepareHostdevs,
.preparePciHostdevs = virPreparePciHostdevs,
.prepareUsbHostdevs = virPrepareUsbHostdevs
.reattachHostdevs = virReattachHostdevs,
.reattachPciHostdevs = virReattachPciHostdevs,
.reattachUsbHostdevs = virReattachUsbHostdevs,
.getActivePciHostdevList = virGetActivePciHostdevList,
.getActiveUsbHostdevList = virGetActiveUsbHostdevList,
.getDomainActivePciHostdevList = virGetDomainActivePciHostdevList,
.getDomainActiveUsbHostdevList = virGetDomainActiveUsbHostdevList,
};
static virStateDriver hostdevStateDriver = {
.name = hostdev,
.initialize = hostdevDriverStartup,
.cleanup = hostdevDriverCleanup,
.reload = hostdevDriverReload,
};
* Changed Definitions:
struct _virPCIDevice {
..
--- const char*used_by; /* The domain which uses the device */
+++ virDomainObjPtr used_by; /* include domname and conn info */
..
};
struct _virUSBDevice {
..
--- const char*used_by; /* name of the domain using this dev */
+++ virDomainObjPtr used_by; /* include domname and conn info */
};
* APIs:
typedef int
(*virDrvPrepareHostdevs)(virHostdevDriverPtr driver,
virDomainObjPtr vm,
unsigned int flags);
/*
- workflow:
call PrepareHostdevPciDevices and PrepareHostdevUsbDevices
to do specific work.
- reference:
int qemuPrepareHostDevices(virQEMUDriverPtr driver,
virDomainDefPtr def,
bool coldBoot);
- new parameter:
- flags:
- could set coldBoot for usb usage
- could set hypervisor tag since for qemu it will use
pci-stub, for libxl, it will use pciback.
*/
typedef int
(*virDrvPreparePciHostdevs)(virHostdevDriverPtr driver,
virDomainObjPtr vm,
[libvirt] [libvirt-sandbox][PATCH] Avoid segfault in gvir_sandbox_config_add_host_include_file
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=924574
Valgrind defects memory error:
==19297== Invalid free() / delete / delete[] / realloc()
==19297==at 0x4A077A6: free (vg_replace_malloc.c:446)
==19297==by 0x350F24D79E: g_free (in /usr/lib64/libglib-2.0.so.0.3400.2)
==19297==by 0x4C2C03F: gvir_sandbox_config_add_host_include_file
(libvirt-sandbox-config.c:1319)
==19297==by 0x401FB7: main (virt-sandbox.c:171)
==19297== Address 0x4f2094c is 12 bytes inside a block of size 18 alloc'd
==19297==at 0x4A0883C: malloc (vg_replace_malloc.c:270)
==19297==by 0x350F24D68E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3400.2)
==19297==by 0x350F263F0B: g_strdup (in /usr/lib64/libglib-2.0.so.0.3400.2)
==19297==by 0x4C2BF95: gvir_sandbox_config_add_host_include_file
(libvirt-sandbox-config.c:1292)
==19297==by 0x401FB7: main (virt-sandbox.c:171)
Signed-off-by: Alex Jia a...@redhat.com
---
libvirt-sandbox/libvirt-sandbox-config.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-config.c
b/libvirt-sandbox/libvirt-sandbox-config.c
index 665a9fb..135eef1 100644
--- a/libvirt-sandbox/libvirt-sandbox-config.c
+++ b/libvirt-sandbox/libvirt-sandbox-config.c
@@ -1285,6 +1285,7 @@ gboolean
gvir_sandbox_config_add_host_include_file(GVirSandboxConfig *config,
error))) {
const gchar *host;
gchar *guest;
+const gchar *relguest;
GVirSandboxConfigMount *mnt = NULL;
GList *mnts = NULL;
gchar *tmp;
@@ -1302,7 +1303,7 @@ gboolean
gvir_sandbox_config_add_host_include_file(GVirSandboxConfig *config,
mnt = GVIR_SANDBOX_CONFIG_MOUNT(mnts-data);
const gchar *target = gvir_sandbox_config_mount_get_target(mnt);
if (g_str_has_prefix(guest, target)) {
-guest = guest + strlen(target);
+relguest = guest + strlen(target);
break;
}
mnt = NULL;
@@ -1315,7 +1316,7 @@ gboolean
gvir_sandbox_config_add_host_include_file(GVirSandboxConfig *config,
return FALSE;
}
-gvir_sandbox_config_mount_add_include(GVIR_SANDBOX_CONFIG_MOUNT(mnt),
host, guest);
+gvir_sandbox_config_mount_add_include(GVIR_SANDBOX_CONFIG_MOUNT(mnt),
host, relguest);
g_free(guest);
g_free(line);
}
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] remote connection issue 'virsh -c qemu+ssh:///root@localhost/system list'
Hi Doug, Thanks for your help. qemu+tcp could work now after enabling listen_tcp in /etc/libvirt/libvirtd.conf. user@x86:~$ virsh -c qemu+tcp://10.193.20.109/system list --all IdName State 2 test running For qemu+ssh, it seemed that we need build standalone netcat instead use nc in busybox. For qemu+tls, we need generate many pem files on both server and client. http://wiki.libvirt.org/page/TLSCreateServerCerts Server: /etc/pki/CA/cacert.pem /etc/pki/libvirt/servercert.pem /etc/pki/libvirt/private/serverkey.pem Client: /etc/pki/CA/cacert.pem /etc/pki/libvirt/clientcert.pem /etc/pki/libvirt/private/clientkey.pem But one question is that how does a client know the server name if without DNS service. Our scenario is a ppc board as server. Could we use IP address as the CN in template file? # cat host1_server_template.info organization = libvirt.org cn = host1 tls_www_server encryption_key signing_key Best Regards, Olivia -Original Message- From: car...@cardoe.com [mailto:car...@cardoe.com] On Behalf Of Doug Goldstein Sent: Friday, March 22, 2013 1:59 AM To: Yin Olivia-R63875 Cc: libvir-list@redhat.com; libvirt-us...@redhat.com Subject: Re: [libvirt] remote connection issue 'virsh -c qemu+ssh:///root@localhost/system list' On Thu, Mar 21, 2013 at 6:23 AM, Yin Olivia-R63875 r63...@freescale.com wrote: Hi, I'm trying remote connection with qemu hypervisor on FSL PPC board. The libvirt server is the PPC board. root@ppc:~# ifconfig eth0 10.193.20.109 root@ppc:~# libvirtd -d root@ppc:~# virsh -c qemu:///system define test.xml root@ppc:~# virsh -c qemu:///system start test root@ppc:~# virsh -c qemu:///system list --all IdName State 2 test running Connect from an X86 PC (Ubuntu 10.04) to the PPC board. user@x86:~$ virsh -c qemu+ssh://root@10.193.20.109/system list --all The authenticity of host '10.193.20.109 (10.193.20.109)' can't be established. RSA key fingerprint is 2f:56:07:08:da:7d:ac:41:45:57:d2:12:15:19:67:e0. Are you sure you want to continue connecting (yes/no)? yes root@10.193.20.109's password: error: failed to connect to the hypervisor error: End of file while reading data: Warning: Permanently added '10.193.20.109' (RSA) to the list of known hosts. nc: invalid option -- 'U' BusyBox v1.19.4 (2013-03-08 13:08:18 CST) multi-call binary. Usage: nc [-iN] [-wN] [-l] [-p PORT] [-f FILE|IPADDR PORT] [-e PROG]: Input/output error I tried to verify the remote connection on localhost. But it also failed as below: root@mpc8572ds:~# virsh -c qemu+ssh:///root@localhost/system list --all root@localhost's password: error: failed to connect to the hypervisor error: End of file while reading data: nc: invalid option -- 'U' BusyBox v1.19.4 (2013-03-08 13:08:18 CST) multi-call binary. Usage: nc [-iN] [-wN] [-l] [-p PORT] [-f FILE|IPADDR PORT] [-e PROG]: Input/output error Could anyone give suggestion on this issue? Best Regards, Olivia You're using busybox's nc (netcat) implementation. It does not support UNIX sockets which is a requirement of libvirt when using the qemu+ssh:// scheme to connect. I'd suggest looking into generating some certificates and using qemu+tls:// (the default when using qemu:// to a remote system). If you don't care about any authentication or encryption for testing purposes you can use qemu+tcp:// -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Fix docs for virsh setmaxmem
The docs assumed the command works always for QEMU and other hypervisors. Unfortunately until qemu will add memory hotplug this can't be done. Fix the docs to mention this limitation. --- tools/virsh.pod | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index b5e632e..07e7c24 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1469,23 +1469,24 @@ paravirtualized or running the PV balloon driver. =item Bsetmaxmem Idomain Bsize [[I--config] [I--live] | [I--current]] Change the maximum memory allocation limit for a guest domain. If I--live is specified, affect a running guest. If I--config is specified, affect the next boot of a persistent guest. If I--current is specified, affect the current guest state. Both I--live and I--config flags may be given, but I--current is exclusive. If no flag is specified, behavior is different depending on hypervisor. -This command works for at least the Xen, QEMU/KVM and vSphere/ESX hypervisors. +Some hypervisors such as QEMU/KVM don't support live changes (especially +increasing) of the maximum memory limit. Isize is a scaled integer (see BNOTES above); it defaults to kibibytes (blocks of 1024 bytes) unless you provide a suffix (and the older option name I--kilobytes is available as a deprecated synonym) . Libvirt rounds up to the nearest kibibyte. Some hypervisors require a larger granularity than KiB, and requests that are not an even multiple will be rounded up. For example, vSphere/ESX rounds the parameter up to mebibytes (1024 kibibytes). =item Bmemtune Idomain [I--hard-limit Bsize] [I--soft-limit Bsize] [I--swap-hard-limit Bsize] [I--min-guarantee Bsize] [[I--config] [I--live] | [I--current]] -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] doc: write separate module for hostdev passthrough and in-use tracking
On 2013年03月22日 17:36, Chunyan Liu wrote:
Hi, List,
As the mail I've sent a week before:
https://www.redhat.com/archives/libvir-list/2013-March/msg00730.html
I'm willing to push this work forward so that the passthrough APIs
could be reused by qemu driver and libxl driver (which doesn't support
pci passthrough yet and tries to add this function recently), or other
drivers.
But since this work affacts a lot, I'm not sure if I can control it in
a correct way. I write a draft to describe what I'm considering how to
do, as in the following and in attachment. Hope to get your review,
comment and guidence to improve the work before start coding. Any
feedback will be very appreciated!
Thanks!
Chunyan
DRAFT:
Write separate module for hostdev passthrough
Cool. we lacked this for a time.
1. Purposes:
* Move hostdev passthrough APIs from qemu_hostdev.ch to separate module so
that they could be reused by other hypervisors too
* Maintain global in-use state of hostdevs
This is more important than the code-reuse.
2. Module design (draft):
* New module name: hostdev_driver
* New module files: hostdev_driver.ch hostdev_conf.ch
* New Definitions:
## [src/driver.h]
typedef struct _virHostdevDriver virHostdevDriver;
typedef virHostdevDriver *virHostdevDriverPtr;
struct _virHosedevDriver {
const char *name;
virDrvOpen open;
virDrvClose close;
virDrvPrepareHostdevsprepareHostdevs;
virDrvPreparePciHostdevspreparePciHostdevs;
virDrvprepareUsbHostdevsprepareUsbHostdevs;
In case of you want to expose prepareHostdevs, no need to expose
preparePciHostdevs and prepareUsbHostdevs?
virDrvReattachHostdevsreattachHostdevs;
virDrvReattachPciHostdevsreattachPciHostdevs;
virDrvReattachUsbHostdevs reattachUsbHostdevs;
Likewise.
virDrvGetActivePciHostdevList getActivePciHostdevList;
virDrvGetActiveUsbHostdevList getActiveUsbHostdevList;
virDrvGetDomainActivePciHostdevList
getDomainActivePciHostdevList;
virDrvGetDomainActiveUsbHostdevList
getDomainActiveUsbHostdevList;
These APIs are useful for upper layer management too. I have once
wanted to create similiar APIs, but only tended for qemu driver
at that time.
But except these 4 get APIs, others are only useful for other drivers
(internally), useless for upper layer management. Do we really want a
driver instead of just an internal share module? Like src/nodeinfo.[ch],
and with it we still can expose APIs like the 4 get APIs.
};
## [src/hostdev/hostdev_conf.h]
typedef struct _virHostdevDriverState virHostdevDriverState;
typedef virHostdevDriverState *virHostdevDriverStatePtr;
struct _virHostdevDriverState {
virMutex lock;
virPCIDeviceListPtr activePciHostdevs;
virPCIDeviceListPtr inactivePciHostdevs;
virUSBDeviceListPtr activeUsbHostdevs;
};
## [src/hostdev/hostdev_driver.c]
static virHostdevDriver hostdevDriver = {
.name = hostdev,
.open = hostdevDriverOpen,
.close = hostdevDriverClose,
.prepareHostdevs = virPrepareHostdevs,
.preparePciHostdevs = virPreparePciHostdevs,
.prepareUsbHostdevs = virPrepareUsbHostdevs
.reattachHostdevs = virReattachHostdevs,
.reattachPciHostdevs = virReattachPciHostdevs,
.reattachUsbHostdevs = virReattachUsbHostdevs,
.getActivePciHostdevList = virGetActivePciHostdevList,
.getActiveUsbHostdevList = virGetActiveUsbHostdevList,
.getDomainActivePciHostdevList = virGetDomainActivePciHostdevList,
.getDomainActiveUsbHostdevList = virGetDomainActiveUsbHostdevList,
};
static virStateDriver hostdevStateDriver = {
.name = hostdev,
.initialize = hostdevDriverStartup,
.cleanup = hostdevDriverCleanup,
.reload = hostdevDriverReload,
};
* Changed Definitions:
struct _virPCIDevice {
..
--- const char*used_by; /* The domain which uses the device */
+++ virDomainObjPtr used_by; /* include domname and conn info */
Why need the conn info? Isn't a driver name enough here?
..
};
struct _virUSBDevice {
..
--- const char*used_by; /* name of the domain using this dev
*/
+++ virDomainObjPtr used_by; /* include domname and conn info */
};
* APIs:
typedef int
(*virDrvPrepareHostdevs)(virHostdevDriverPtr driver,
[libvirt] [PATCH] qemu: Don't set type too earla during virtio disk hotplug
f946462e14ac036357b7c11ce5c23f94a3ee4e49 changed behaviour by settings
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI upfront. If we do so before invoking
qemuDomainPCIAddressEnsureAddr we merely try to set the PCI slot via
qemuDomainPCIAddressReserveSlot instead reserving a new address via
qemuDomainPCIAddressSetNextAddr which fails with
$ ~/run-tck-test domain/200-disk-hotplug.t
./scripts/domain/200-disk-hotplug.t .. # Creating a new transient domain
./scripts/domain/200-disk-hotplug.t .. 1/5 # Attaching the new disk
/var/lib/jenkins/jobs/libvirt-tck-build/workspace/scratchdir/200-disk-hotplug/extra.img
# Failed test 'disk has been attached'
# at ./scripts/domain/200-disk-hotplug.t line 67.
# died: Sys::Virt::Error (libvirt error code: 1, message: internal error
unable to reserve PCI address 0:0:0.0
# )
---
This unbreaks libvirt-tck on debian wheezy.
Cheers,
-- Guido
src/qemu/qemu_hotplug.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 044f8cd..576738b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -226,7 +226,6 @@ int qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
else if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_VIRTIO_S390))
disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
-else disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
}
for (i = 0 ; i vm-def-ndisks ; i++) {
@@ -253,7 +252,8 @@ int qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (qemuDomainCCWAddressAssign(disk-info, priv-ccwaddrs,
!disk-info.addr.ccw.assigned) 0)
goto error;
-} else if (disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+} else if (!disk-info.type ||
+disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
if (qemuDomainPCIAddressEnsureAddr(priv-pciaddrs, disk-info)
0)
goto error;
}
@@ -291,14 +291,17 @@ int qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
}
}
}
-} else if (disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI){
+} else if (!disk-info.type ||
+disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
virDevicePCIAddress guestAddr = disk-info.addr.pci;
ret = qemuMonitorAddPCIDisk(priv-mon,
disk-src,
type,
guestAddr);
-if (ret == 0)
+if (ret == 0) {
+disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
memcpy(disk-info.addr.pci, guestAddr, sizeof(guestAddr));
+}
}
qemuDomainObjExitMonitor(driver, vm);
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer] Add GvirDesignerDomain::osinfo-db
virtxml was doing its own loading of the libosinfo database,
and gvir_designer_init() was loading it a second time.
By adding a GVirDesignerDomain::osinfo_db property, applications
can share the same OsinfoDb as libvirt-designer. The association
is made per libvirt-designer domain for more flexibility.
---
examples/virtxml.c | 2 +-
libvirt-designer/libvirt-designer-domain.c | 34 ++--
libvirt-designer/libvirt-designer-domain.h | 3 ++-
libvirt-designer/libvirt-designer-internal.h | 3 ---
libvirt-designer/libvirt-designer-main.c | 12 --
libvirt-designer/libvirt-designer-main.h | 2 ++
6 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/examples/virtxml.c b/examples/virtxml.c
index a68843d..c5a5e24 100644
--- a/examples/virtxml.c
+++ b/examples/virtxml.c
@@ -558,7 +558,7 @@ main(int argc, char *argv[])
goto cleanup;
}
-domain = gvir_designer_domain_new(os, platform, caps);
+domain = gvir_designer_domain_new(db, os, platform, caps);
gvir_designer_domain_setup_machine(domain, error);
CHECK_ERROR;
diff --git a/libvirt-designer/libvirt-designer-domain.c
b/libvirt-designer/libvirt-designer-domain.c
index 3e31bd1..49e8068 100644
--- a/libvirt-designer/libvirt-designer-domain.c
+++ b/libvirt-designer/libvirt-designer-domain.c
@@ -35,6 +35,7 @@ struct _GVirDesignerDomainPrivate
{
GVirConfigDomain *config;
GVirConfigCapabilities *caps;
+OsinfoDb *osinfo_db;
OsinfoOs *os;
OsinfoPlatform *platform;
@@ -66,6 +67,7 @@ enum {
PROP_OS,
PROP_PLATFORM,
PROP_CAPS,
+PROP_OSINFO_DB,
};
static void gvir_designer_domain_get_property(GObject *object,
@@ -83,6 +85,10 @@ static void gvir_designer_domain_get_property(GObject
*object,
g_value_set_object(value, priv-config);
break;
+case PROP_OSINFO_DB:
+g_value_set_object(value, priv-osinfo_db);
+break;
+
case PROP_OS:
g_value_set_object(value, priv-os);
break;
@@ -112,6 +118,11 @@ static void gvir_designer_domain_set_property(GObject
*object,
GVirDesignerDomainPrivate *priv = design-priv;
switch (prop_id) {
+case PROP_OSINFO_DB:
+if (priv-osinfo_db)
+g_object_unref(priv-osinfo_db);
+priv-osinfo_db = g_value_dup_object(value);
+break;
case PROP_OS:
if (priv-os)
g_object_unref(priv-os);
@@ -147,6 +158,8 @@ static void gvir_designer_domain_finalize(GObject *object)
g_object_unref(priv-caps);
if (priv-deployment)
g_object_unref(priv-deployment);
+if (priv-osinfo_db)
+g_object_unref(priv-osinfo_db);
G_OBJECT_CLASS(gvir_designer_domain_parent_class)-finalize(object);
}
@@ -171,6 +184,16 @@ static void
gvir_designer_domain_class_init(GVirDesignerDomainClass *klass)
G_PARAM_STATIC_STRINGS));
g_object_class_install_property(object_class,
+PROP_OSINFO_DB,
+g_param_spec_object(osinfo-db,
+Osinfo Database,
+libosinfo database,
+OSINFO_TYPE_DB,
+G_PARAM_READABLE |
+G_PARAM_WRITABLE |
+G_PARAM_CONSTRUCT_ONLY
|
+
G_PARAM_STATIC_STRINGS));
+g_object_class_install_property(object_class,
PROP_OS,
g_param_spec_object(os,
Os,
@@ -215,11 +238,13 @@ static void gvir_designer_domain_init(GVirDesignerDomain
*design)
}
-GVirDesignerDomain *gvir_designer_domain_new(OsinfoOs *os,
+GVirDesignerDomain *gvir_designer_domain_new(OsinfoDb *db,
+ OsinfoOs *os,
OsinfoPlatform *platform,
GVirConfigCapabilities *caps)
{
return GVIR_DESIGNER_DOMAIN(g_object_new(GVIR_DESIGNER_TYPE_DOMAIN,
+ osinfo-db, db,
os, os,
platform, platform,
capabilities, caps,
@@ -721,7 +746,12 @@
gvir_designer_domain_get_preferred_device(GVirDesignerDomain *design,
OsinfoDeviceLink *dev_link = NULL;
if (!deployment) {
-priv-deployment = deployment = osinfo_db_find_deployment(osinfo_db,
+if (!priv-osinfo_db) {
+g_set_error(error, GVIR_DESIGNER_DOMAIN_ERROR, 0,
+
[libvirt] [PATCH] viralloc: Export virAllocTest*
If users build with --enable-test-oom configure option,
they get this error saying, virAllocTest* functions are
not defined within tests/testutils.c.
---
src/libvirt_private.syms | 4
src/util/viralloc.c | 24
src/util/viralloc.h | 7 ---
tests/testutils.c| 2 +-
4 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9529265..f241ec4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -993,6 +993,10 @@ virSecurityManagerVerify;
# util/viralloc.h
virAlloc;
virAllocN;
+virAllocTestCount;
+virAllocTestHook;
+virAllocTestInit;
+virAllocTestOOM;
virAllocVar;
virDeleteElementsN;
virExpandN;
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index 807de04..342b0eb 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -74,6 +74,30 @@ static int virAllocTestFail(void)
testMallocNext++;
return fail;
}
+
+#else
+
+void virAllocTestOOM(int n ATTRIBUTE_UNUSED,
+ int m ATTRIBUTE_UNUSED)
+{
+/* nada */
+}
+
+int virAllocTestCount(void)
+{
+return 0;
+}
+
+void virAllocTestInit(void)
+{
+/* nada */
+}
+
+void virAllocTestHook(void (*func)(int, void*) ATTRIBUTE_UNUSED,
+ void *data ATTRIBUTE_UNUSED)
+{
+/* nada */
+}
#endif
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 6f46d0b..7be7f82 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -376,15 +376,8 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
# define VIR_FREE(ptr) virFree((void *) (ptr))
# endif
-
-
-# if TEST_OOM
void virAllocTestInit(void);
int virAllocTestCount(void);
void virAllocTestOOM(int n, int m);
void virAllocTestHook(void (*func)(int, void*), void *data);
-# endif
-
-
-
#endif /* __VIR_MEMORY_H_ */
diff --git a/tests/testutils.c b/tests/testutils.c
index ea46c09..0fb69ec 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -697,7 +697,7 @@ int virtTestMain(int argc,
if (worker) {
_exit(ret);
} else {
-int i, status;
+int i;
for (i = 0 ; i mp ; i++) {
if (virProcessWait(workers[i], NULL) 0)
ret = EXIT_FAILURE;
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer] Add GvirDesignerDomain::osinfo-db
On 22.03.2013 11:11, Christophe Fergeau wrote: virtxml was doing its own loading of the libosinfo database, and gvir_designer_init() was loading it a second time. By adding a GVirDesignerDomain::osinfo_db property, applications can share the same OsinfoDb as libvirt-designer. The association is made per libvirt-designer domain for more flexibility. --- examples/virtxml.c | 2 +- libvirt-designer/libvirt-designer-domain.c | 34 ++-- libvirt-designer/libvirt-designer-domain.h | 3 ++- libvirt-designer/libvirt-designer-internal.h | 3 --- libvirt-designer/libvirt-designer-main.c | 12 -- libvirt-designer/libvirt-designer-main.h | 2 ++ 6 files changed, 37 insertions(+), 19 deletions(-) ACK, the global variables look really ugly. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer] Add GvirDesignerDomain::osinfo-db
On Fri, Mar 22, 2013 at 11:22:20AM +0100, Michal Privoznik wrote: On 22.03.2013 11:11, Christophe Fergeau wrote: virtxml was doing its own loading of the libosinfo database, and gvir_designer_init() was loading it a second time. By adding a GVirDesignerDomain::osinfo_db property, applications can share the same OsinfoDb as libvirt-designer. The association is made per libvirt-designer domain for more flexibility. --- examples/virtxml.c | 2 +- libvirt-designer/libvirt-designer-domain.c | 34 ++-- libvirt-designer/libvirt-designer-domain.h | 3 ++- libvirt-designer/libvirt-designer-internal.h | 3 --- libvirt-designer/libvirt-designer-main.c | 12 -- libvirt-designer/libvirt-designer-main.h | 2 ++ 6 files changed, 37 insertions(+), 19 deletions(-) ACK, the global variables look really ugly. Fwiw, I forgot to tag this as PATCHv3, this is the 3rd iteration of this patch. Christophe pgpuHkj3sRQtg.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-sandbox][PATCH] Avoid segfault in gvir_sandbox_config_add_host_include_file
On Fri, Mar 22, 2013 at 05:38:23PM +0800, Alex Jia wrote:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=924574
Valgrind defects memory error:
==19297== Invalid free() / delete / delete[] / realloc()
==19297==at 0x4A077A6: free (vg_replace_malloc.c:446)
==19297==by 0x350F24D79E: g_free (in /usr/lib64/libglib-2.0.so.0.3400.2)
==19297==by 0x4C2C03F: gvir_sandbox_config_add_host_include_file
(libvirt-sandbox-config.c:1319)
==19297==by 0x401FB7: main (virt-sandbox.c:171)
==19297== Address 0x4f2094c is 12 bytes inside a block of size 18 alloc'd
==19297==at 0x4A0883C: malloc (vg_replace_malloc.c:270)
==19297==by 0x350F24D68E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3400.2)
==19297==by 0x350F263F0B: g_strdup (in /usr/lib64/libglib-2.0.so.0.3400.2)
==19297==by 0x4C2BF95: gvir_sandbox_config_add_host_include_file
(libvirt-sandbox-config.c:1292)
==19297==by 0x401FB7: main (virt-sandbox.c:171)
Signed-off-by: Alex Jia a...@redhat.com
---
libvirt-sandbox/libvirt-sandbox-config.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/libvirt-sandbox/libvirt-sandbox-config.c
b/libvirt-sandbox/libvirt-sandbox-config.c
index 665a9fb..135eef1 100644
--- a/libvirt-sandbox/libvirt-sandbox-config.c
+++ b/libvirt-sandbox/libvirt-sandbox-config.c
@@ -1285,6 +1285,7 @@ gboolean
gvir_sandbox_config_add_host_include_file(GVirSandboxConfig *config,
error))) {
const gchar *host;
gchar *guest;
+const gchar *relguest;
GVirSandboxConfigMount *mnt = NULL;
GList *mnts = NULL;
gchar *tmp;
@@ -1302,7 +1303,7 @@ gboolean
gvir_sandbox_config_add_host_include_file(GVirSandboxConfig *config,
mnt = GVIR_SANDBOX_CONFIG_MOUNT(mnts-data);
const gchar *target = gvir_sandbox_config_mount_get_target(mnt);
if (g_str_has_prefix(guest, target)) {
-guest = guest + strlen(target);
+relguest = guest + strlen(target);
break;
}
I think you need to have
} else {
relguest = guest;
}
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer] Add GvirDesignerDomain::osinfo-db
On Fri, Mar 22, 2013 at 11:11:30AM +0100, Christophe Fergeau wrote: virtxml was doing its own loading of the libosinfo database, and gvir_designer_init() was loading it a second time. By adding a GVirDesignerDomain::osinfo_db property, applications can share the same OsinfoDb as libvirt-designer. The association is made per libvirt-designer domain for more flexibility. --- examples/virtxml.c | 2 +- libvirt-designer/libvirt-designer-domain.c | 34 ++-- libvirt-designer/libvirt-designer-domain.h | 3 ++- libvirt-designer/libvirt-designer-internal.h | 3 --- libvirt-designer/libvirt-designer-main.c | 12 -- libvirt-designer/libvirt-designer-main.h | 2 ++ 6 files changed, 37 insertions(+), 19 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] viralloc: Export virAllocTest*
On Fri, Mar 22, 2013 at 11:15:14AM +0100, Michal Privoznik wrote: If users build with --enable-test-oom configure option, they get this error saying, virAllocTest* functions are not defined within tests/testutils.c. --- src/libvirt_private.syms | 4 src/util/viralloc.c | 24 src/util/viralloc.h | 7 --- tests/testutils.c| 2 +- 4 files changed, 29 insertions(+), 8 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix free of uninitialized value in LXC numad setup
From: Daniel P. Berrange berra...@redhat.com
The 'nodeset' variable was never initialized, causing a later
VIR_FREE(nodeset) to free uninitialized memory.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_controller.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Pushed under trivial rule
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 37e3ce9..bb369e2 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -517,7 +517,7 @@ static int
virLXCControllerGetNumadAdvice(virLXCControllerPtr ctrl,
virBitmapPtr *mask)
{
virBitmapPtr nodemask = NULL;
-char *nodeset;
+char *nodeset = NULL;
int ret = -1;
/* Get the advisory nodeset from numad if 'placement' of
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 0/2] Report OOM on VIR_ALLOC failure
Currently, our code is plenty of following scheme:
if (VIR_ALLOC(dummyPtr) 0) {
virReportOOMError();
goto cleanup;
}
or something similar. What if we just move the OOM reporting into
VIR_ALLOC? It would have three nice features:
1) sizeof(code base) gets lower. A lot lower.
2) even for callers which don't follow the schema described
above, there is no harm reporting so serious error in the
logs. No matter that the callee may fall back and return
success.
3) Removing virReportOOMError() from the schema does not need to
be done at once, but can be split into several patches. In the
worst case scenario - the error gets reported twice.
But before I start working on other areas of code, I want to make
sure there's an agreement if this is even desired. As an example,
how much the code base will lose on weight, I've done the
conversion under src/util/:
30 files changed, 81 insertions(+), 221 deletions(-)
Michal Privoznik (2):
viralloc: Report OOM error on failure
util: Don't report OOM twice
src/util/iohelper.c | 4 +---
src/util/viralloc.c | 23 ++-
src/util/viralloc.h | 13 -
src/util/virauthconfig.c | 8 ++--
src/util/vircommand.c| 13 +++--
src/util/virconf.c | 10 ++
src/util/virdnsmasq.c| 19 +++
src/util/virerror.c | 2 +-
src/util/vireventpoll.c | 4 +---
src/util/virfile.c | 4 +---
src/util/virhash.c | 10 ++
src/util/virkeyfile.c| 4 +---
src/util/virlockspace.c | 11 +++
src/util/virnetdev.c | 4 +---
src/util/virnetdevbandwidth.c| 12 +++-
src/util/virnetdevmacvlan.c | 4 ++--
src/util/virnetdevvlan.c | 4 +---
src/util/virnetdevvportprofile.c | 4 +---
src/util/virnetlink.c| 8 ++--
src/util/virobject.c | 9 -
src/util/virpci.c| 13 +++--
src/util/virprocess.c| 4 +---
src/util/virsexpr.c | 4 +---
src/util/virstoragefile.c| 24 ++--
src/util/virstring.c | 5 +++--
src/util/virsysinfo.c| 8 +++-
src/util/virthreadpool.c | 21 +
src/util/virtime.c | 8 ++--
src/util/virtypedparam.c | 36 +---
src/util/viruri.c| 2 +-
src/util/virusb.c| 8 ++--
src/util/virutil.c | 36
src/util/virxml.c| 1 -
33 files changed, 108 insertions(+), 232 deletions(-)
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 2/2] util: Don't report OOM twice
Adapt code under src/util/ to fact, that VIR_ALLOC* now reports OOM
error. There is no need to report it twice now.
---
src/util/iohelper.c | 4 +---
src/util/virauthconfig.c | 8 ++--
src/util/vircommand.c| 13 +++--
src/util/virconf.c | 10 ++
src/util/virdnsmasq.c| 19 +++
src/util/vireventpoll.c | 4 +---
src/util/virfile.c | 4 +---
src/util/virhash.c | 10 ++
src/util/virkeyfile.c| 4 +---
src/util/virlockspace.c | 11 +++
src/util/virnetdev.c | 4 +---
src/util/virnetdevbandwidth.c| 12 +++-
src/util/virnetdevmacvlan.c | 4 ++--
src/util/virnetdevvlan.c | 4 +---
src/util/virnetdevvportprofile.c | 4 +---
src/util/virnetlink.c| 8 ++--
src/util/virobject.c | 9 -
src/util/virpci.c| 13 +++--
src/util/virprocess.c| 4 +---
src/util/virsexpr.c | 4 +---
src/util/virstoragefile.c| 24 ++--
src/util/virstring.c | 5 +++--
src/util/virsysinfo.c| 8 +++-
src/util/virthreadpool.c | 21 +
src/util/virtime.c | 8 ++--
src/util/virtypedparam.c | 36 +---
src/util/viruri.c| 2 +-
src/util/virusb.c| 8 ++--
src/util/virutil.c | 36
src/util/virxml.c| 1 -
30 files changed, 81 insertions(+), 221 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 2230bcb..d3c966a 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -94,10 +94,8 @@ runIO(const char *path, int fd, int oflags, unsigned long
long length)
}
buf = base;
#else
-if (VIR_ALLOC_N(buf, buflen + alignMask) 0) {
-virReportOOMError();
+if (VIR_ALLOC_N(buf, buflen + alignMask) 0)
goto cleanup;
-}
base = buf;
buf = (char *) (((intptr_t) base + alignMask) ~alignMask);
#endif
diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index 1d1f084..8847d9e 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -43,10 +43,8 @@ virAuthConfigPtr virAuthConfigNew(const char *path)
{
virAuthConfigPtr auth;
-if (VIR_ALLOC(auth) 0) {
-virReportOOMError();
+if (VIR_ALLOC(auth) 0)
goto error;
-}
if (!(auth-path = strdup(path))) {
virReportOOMError();
@@ -73,10 +71,8 @@ virAuthConfigPtr virAuthConfigNewData(const char *path,
{
virAuthConfigPtr auth;
-if (VIR_ALLOC(auth) 0) {
-virReportOOMError();
+if (VIR_ALLOC(auth) 0)
goto error;
-}
if (!(auth-path = strdup(path))) {
virReportOOMError();
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index ac56a63..052f1e7 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1804,17 +1804,13 @@ virCommandProcessIO(virCommandPtr cmd)
* results accumulated over a prior run of the same command. */
if (cmd-outbuf) {
outfd = cmd-outfd;
-if (VIR_REALLOC_N(*cmd-outbuf, 1) 0) {
-virReportOOMError();
+if (VIR_REALLOC_N(*cmd-outbuf, 1) 0)
ret = -1;
-}
}
if (cmd-errbuf) {
errfd = cmd-errfd;
-if (VIR_REALLOC_N(*cmd-errbuf, 1) 0) {
-virReportOOMError();
+if (VIR_REALLOC_N(*cmd-errbuf, 1) 0)
ret = -1;
-}
}
if (ret == -1)
goto cleanup;
@@ -1888,10 +1884,8 @@ virCommandProcessIO(virCommandPtr cmd)
else
errfd = -1;
} else {
-if (VIR_REALLOC_N(*buf, *len + done + 1) 0) {
-virReportOOMError();
+if (VIR_REALLOC_N(*buf, *len + done + 1) 0)
goto cleanup;
-}
memcpy(*buf + *len, data, done);
*len += done;
}
@@ -2435,7 +2429,6 @@ int virCommandHandshakeWait(virCommandPtr cmd)
char *msg;
ssize_t len;
if (VIR_ALLOC_N(msg, 1024) 0) {
-virReportOOMError();
VIR_FORCE_CLOSE(cmd-handshakeWait[0]);
return -1;
}
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 16f074a..1c7075f 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -173,10 +173,8 @@ virConfNew(void)
{
virConfPtr ret;
-if (VIR_ALLOC(ret) 0) {
-virReportOOMError();
+if (VIR_ALLOC(ret) 0)
return NULL;
-}
ret-filename = NULL;
ret-flags = 0;
@@ -225,10 +223,8 @@ virConfAddEntry(virConfPtr conf, char *name,
virConfValuePtr value, char *comm)
if ((comm == NULL) (name == NULL))
return
[libvirt] [PATCH RFC 1/2] viralloc: Report OOM error on failure
In nearly all cases of calling VIR_ALLOC*, VIR_REALLOC_N,
VIR_EXPAND_N, VIR_RESIZE_N, VIR_*_ELEMENT etc. we want to
report OOM error, so our source code base is full of:
if (VIR_ALLOC(somePtr) 0) {
virReportOOMError();
goto cleanup;
}
or similar. Moreover, for those few cases where we don't want to
report OOM error (e.g. virReportOOMError() itself) a new
VIR_ALLOC_NOOOM macro is being introduced.
---
src/util/viralloc.c | 23 ++-
src/util/viralloc.h | 13 -
src/util/virerror.c | 2 +-
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index 342b0eb..60c33d2 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -24,8 +24,11 @@
#include stdlib.h
#include viralloc.h
+#include virerror.h
#include virlog.h
+#define VIR_FROM_THIS VIR_FROM_NONE
+
#if TEST_OOM
static int testMallocNext = 0;
static int testMallocFailFirst = 0;
@@ -105,6 +108,7 @@ void virAllocTestHook(void (*func)(int, void*)
ATTRIBUTE_UNUSED,
* virAlloc:
* @ptrptr: pointer to pointer for address of allocated memory
* @size: number of bytes to allocate
+ * @report: report OOM error
*
* Allocate 'size' bytes of memory. Return the address of the
* allocated memory in 'ptrptr'. The newly allocated memory is
@@ -112,7 +116,7 @@ void virAllocTestHook(void (*func)(int, void*)
ATTRIBUTE_UNUSED,
*
* Returns -1 on failure to allocate, zero on success
*/
-int virAlloc(void *ptrptr, size_t size)
+int virAlloc(void *ptrptr, size_t size, bool report)
{
#if TEST_OOM
if (virAllocTestFail()) {
@@ -122,8 +126,11 @@ int virAlloc(void *ptrptr, size_t size)
#endif
*(void **)ptrptr = calloc(1, size);
-if (*(void **)ptrptr == NULL)
+if (*(void **)ptrptr == NULL) {
+if (report)
+virReportOOMError();
return -1;
+}
return 0;
}
@@ -150,8 +157,10 @@ int virAllocN(void *ptrptr, size_t size, size_t count)
#endif
*(void**)ptrptr = calloc(count, size);
-if (*(void**)ptrptr == NULL)
+if (*(void**)ptrptr == NULL) {
+virReportOOMError();
return -1;
+}
return 0;
}
@@ -182,8 +191,10 @@ int virReallocN(void *ptrptr, size_t size, size_t count)
return -1;
}
tmp = realloc(*(void**)ptrptr, size * count);
-if (!tmp (size * count))
+if (!tmp (size * count)) {
+virReportOOMError();
return -1;
+}
*(void**)ptrptr = tmp;
return 0;
}
@@ -422,8 +433,10 @@ int virAllocVar(void *ptrptr, size_t struct_size, size_t
element_size, size_t co
alloc_size = struct_size + (element_size * count);
*(void **)ptrptr = calloc(1, alloc_size);
-if (*(void **)ptrptr == NULL)
+if (*(void **)ptrptr == NULL) {
+virReportOOMError();
return -1;
+}
return 0;
}
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7be7f82..30ffe15 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -46,7 +46,7 @@
/* Don't call these directly - use the macros below */
-int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK
+int virAlloc(void *ptrptr, size_t size, bool report) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
@@ -76,13 +76,16 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* VIR_ALLOC:
* @ptr: pointer to hold address of allocated memory
*
- * Allocate sizeof(*ptr) bytes of memory and store
- * the address of allocated memory in 'ptr'. Fill the
- * newly allocated memory with zeros.
+ * Allocate sizeof(*ptr) bytes of memory and store the
+ * address of allocated memory in 'ptr'. Fill the newly
+ * allocated memory with zeros. If there's a failure,
+ * OOM error is reported. The VIR_ALLOC_NOOOM macro
+ * behaves the same except the OOM error reporting.
*
* Returns -1 on failure, 0 on success
*/
-# define VIR_ALLOC(ptr) virAlloc((ptr), sizeof(*(ptr)))
+# define VIR_ALLOC(ptr) virAlloc((ptr), sizeof(*(ptr)), true)
+# define VIR_ALLOC_NOOOM(ptr) virAlloc((ptr), sizeof(*(ptr)), true)
/**
* VIR_ALLOC_N:
diff --git a/src/util/virerror.c b/src/util/virerror.c
index c30642a..c033129 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -204,7 +204,7 @@ virLastErrorObject(void)
virErrorPtr err;
err = virThreadLocalGet(virLastErr);
if (!err) {
-if (VIR_ALLOC(err) 0)
+if (VIR_ALLOC_NOOOM(err) 0)
return NULL;
if (virThreadLocalSet(virLastErr, err) 0)
VIR_FREE(err);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: don't print --(null) in vol-name and vol-pool
Don't print the pool option name if it's null.
Before:
virsh # vol-name vol
error: failed to get vol 'vol', specifying --(null) might help
error: Storage volume not found: no storage vol with matching path vol
After:
virsh # vol-name vol
error: failed to get vol 'vol'
error: Storage volume not found: no storage vol with matching path vol
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=924571
---
tools/virsh-volume.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 89ad8ea..0ca295f 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -88,7 +88,7 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
}
if (!vol) {
-if (pool)
+if (pool || !pooloptname)
vshError(ctl, _(failed to get vol '%s'), n);
else
vshError(ctl, _(failed to get vol '%s', specifying --%s
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: don't print --(null) in vol-name and vol-pool
On 22.03.2013 12:51, Ján Tomko wrote:
Don't print the pool option name if it's null.
Before:
virsh # vol-name vol
error: failed to get vol 'vol', specifying --(null) might help
error: Storage volume not found: no storage vol with matching path vol
After:
virsh # vol-name vol
error: failed to get vol 'vol'
error: Storage volume not found: no storage vol with matching path vol
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=924571
---
tools/virsh-volume.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 89ad8ea..0ca295f 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -88,7 +88,7 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
}
if (!vol) {
-if (pool)
+if (pool || !pooloptname)
vshError(ctl, _(failed to get vol '%s'), n);
else
vshError(ctl, _(failed to get vol '%s', specifying --%s
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Problems with filesystem type='block'
On Thu, Mar 21, 2013 at 11:50:40PM -0500, Doug Goldstein wrote: So still trying to figure out what I'm doing wrong with LXC because I just can't get any joy. So I'll go one issue at a time. The following VM definition: domain type='lxc' nametestdeb/name uuiddf03b2ce-725a-42e2-39e4-d646be8facb3/uuid memory unit='KiB'332768/memory currentMemory unit='KiB'332768/currentMemory vcpu placement='static'1/vcpu os type arch='x86_64'exe/type init/sbin/init/init /os clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashdestroy/on_crash devices emulator/usr/libexec/libvirt_lxc/emulator filesystem type='block' accessmode='passthrough' source dev='/dev/mapper/vms-testdeb'/ target dir='/'/ /filesystem So, it seems I only tested 'type=block with non-/ filesystem mounts. I can confirm it is broken for target dir=//. The fix is not exactly trivial, so might take me a while Now if I do the following: # mkdir /mnt/testdeb # mount /dev/mapper/vms-testdeb /mnt/testdeb And change the definition to: filesystem type='mount' accessmode='passthrough' source dir='/mnt/testdeb'/ target dir='/'/ /filesystem It at least appears to work. The LXC domain boots but believes it only has R/O access to its /. What does /proc/mounts say inside the container for '/' ? When I do this I get a full R+W filesystem in the container Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: fix virAllocVar's comment
--- Pushed under the trivial rule. src/util/viralloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/viralloc.c b/src/util/viralloc.c index 342b0eb..8f219bf 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -389,7 +389,7 @@ virDeleteElementsN(void *ptrptr, size_t size, size_t at, } /** - * Vir_Alloc_Var: + * virAllocVar: * @ptrptr: pointer to hold address of allocated memory * @struct_size: size of initial struct * @element_size: size of array elements -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Remove bogus filtering from virDomainGetRootFilesystem
From: Daniel P. Berrange berra...@redhat.com
The virDomainGetRootFilesystem was only returning filesystems
with type=mount. This is bogus - any type of filesystem is
valid as the root, if dst=/.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/domain_conf.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ef67be..4cae0d3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15635,9 +15635,6 @@ virDomainGetRootFilesystem(virDomainDefPtr def)
int i;
for (i = 0 ; i def-nfss ; i++) {
-if (def-fss[i]-type != VIR_DOMAIN_FS_TYPE_MOUNT)
-continue;
-
if (STREQ(def-fss[i]-dst, /))
return def-fss[i];
}
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix thread safety in LXC callback handling
From: Daniel P. Berrange berra...@redhat.com
Some of the LXC callbacks did not lock the virDomainObjPtr
instance. This caused transient errors like
error: Failed to start domain busy-mount
error: cannot rename file '/var/run/libvirt/lxc/busy-mount.xml.new' as
'/var/run/libvirt/lxc/busy-mount.xml': No such file or directory
as 2 threads tried to update the status file concurrently
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_process.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 670a032..39a6ea2 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -610,8 +610,13 @@ static void
virLXCProcessMonitorExitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED
virLXCMonitorExitStatus status,
virDomainObjPtr vm)
{
+virLXCDriverPtr driver = lxc_driver;
virLXCDomainObjPrivatePtr priv = vm-privateData;
+lxcDriverLock(driver);
+virObjectLock(vm);
+lxcDriverUnlock(driver);
+
switch (status) {
case VIR_LXC_MONITOR_EXIT_STATUS_SHUTDOWN:
priv-stopReason = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN;
@@ -629,6 +634,8 @@ static void virLXCProcessMonitorExitNotify(virLXCMonitorPtr
mon ATTRIBUTE_UNUSED
}
VIR_DEBUG(Domain shutoff reason %d (from status %d),
priv-stopReason, status);
+
+virObjectUnlock(vm);
}
static int
@@ -667,9 +674,15 @@ static void
virLXCProcessMonitorInitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED
pid_t initpid,
virDomainObjPtr vm)
{
-virLXCDomainObjPrivatePtr priv = vm-privateData;
+virLXCDriverPtr driver = lxc_driver;
+virLXCDomainObjPrivatePtr priv;
ino_t inode;
+lxcDriverLock(driver);
+virObjectLock(vm);
+lxcDriverUnlock(driver);
+
+priv = vm-privateData;
priv-initpid = initpid;
if (virLXCProcessGetNsInode(initpid, pid, inode) 0) {
@@ -684,6 +697,8 @@ static void virLXCProcessMonitorInitNotify(virLXCMonitorPtr
mon ATTRIBUTE_UNUSED
if (virDomainSaveStatus(lxc_driver-xmlconf, lxc_driver-stateDir, vm) 0)
VIR_WARN(Cannot update XML with PID for LXC %s, vm-def-name);
+
+virObjectUnlock(vm);
}
static virLXCMonitorCallbacks monitorCallbacks = {
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: don't print --(null) in vol-name and vol-pool
On 03/22/2013 12:58 PM, Michal Privoznik wrote: On 22.03.2013 12:51, Ján Tomko wrote: Don't print the pool option name if it's null. Before: virsh # vol-name vol error: failed to get vol 'vol', specifying --(null) might help error: Storage volume not found: no storage vol with matching path vol After: virsh # vol-name vol error: failed to get vol 'vol' error: Storage volume not found: no storage vol with matching path vol Bug: https://bugzilla.redhat.com/show_bug.cgi?id=924571 --- tools/virsh-volume.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK Michal Thanks, pushed now. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove bogus filtering from virDomainGetRootFilesystem
On 22.03.2013 13:10, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The virDomainGetRootFilesystem was only returning filesystems
with type=mount. This is bogus - any type of filesystem is
valid as the root, if dst=/.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/conf/domain_conf.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ef67be..4cae0d3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15635,9 +15635,6 @@ virDomainGetRootFilesystem(virDomainDefPtr def)
int i;
for (i = 0 ; i def-nfss ; i++) {
-if (def-fss[i]-type != VIR_DOMAIN_FS_TYPE_MOUNT)
-continue;
-
if (STREQ(def-fss[i]-dst, /))
return def-fss[i];
}
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Build failed in Jenkins: libvirt-syntax-check #759
On 03/21/2013 09:54 PM, Jenkins CI wrote: See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/759/ -- Started by upstream project libvirt-build build number 856 [workspace] $ /bin/sh -xe /tmp/hudson8330114436432330964.sh + make syntax-check GENbracket-spacing-check GFDL_version preprocessor_indentation maint.mk: skipping test sc_preprocessor_indentation: cppi not installed 0.06 preprocessor_indentation Any chance we can install cppi on the Jenkins machine? prohibit_argmatch_without_use grep: write error grep: write error sed: couldn't write 1 item to stdout: Broken pipe sed: couldn't write 49 items to stdout: Broken pipe 0.58 prohibit_argmatch_without_use There were quite a few of these errors listed; it sounds like it is a bug in the upstream gnulib maint.mk file. Can you help me pinpoint what is going wrong with the syntax check, so we can fix gnulib and make it quieter? Is it just a case of running with SIGPIPE ignored, and hitting a pipeline where the sink of the pipe is exiting early, causing the source to hit EPIPE? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix thread safety in LXC callback handling
On 22.03.2013 13:11, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Some of the LXC callbacks did not lock the virDomainObjPtr
instance. This caused transient errors like
error: Failed to start domain busy-mount
error: cannot rename file '/var/run/libvirt/lxc/busy-mount.xml.new' as
'/var/run/libvirt/lxc/busy-mount.xml': No such file or directory
as 2 threads tried to update the status file concurrently
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_process.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 670a032..39a6ea2 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -610,8 +610,13 @@ static void
virLXCProcessMonitorExitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED
virLXCMonitorExitStatus status,
virDomainObjPtr vm)
{
+virLXCDriverPtr driver = lxc_driver;
virLXCDomainObjPrivatePtr priv = vm-privateData;
+lxcDriverLock(driver);
+virObjectLock(vm);
+lxcDriverUnlock(driver);
+
switch (status) {
case VIR_LXC_MONITOR_EXIT_STATUS_SHUTDOWN:
priv-stopReason = VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN;
@@ -629,6 +634,8 @@ static void
virLXCProcessMonitorExitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED
}
VIR_DEBUG(Domain shutoff reason %d (from status %d),
priv-stopReason, status);
+
+virObjectUnlock(vm);
}
static int
@@ -667,9 +674,15 @@ static void
virLXCProcessMonitorInitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED
pid_t initpid,
virDomainObjPtr vm)
{
-virLXCDomainObjPrivatePtr priv = vm-privateData;
+virLXCDriverPtr driver = lxc_driver;
+virLXCDomainObjPrivatePtr priv;
ino_t inode;
+lxcDriverLock(driver);
+virObjectLock(vm);
+lxcDriverUnlock(driver);
+
+priv = vm-privateData;
priv-initpid = initpid;
if (virLXCProcessGetNsInode(initpid, pid, inode) 0) {
@@ -684,6 +697,8 @@ static void
virLXCProcessMonitorInitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED
if (virDomainSaveStatus(lxc_driver-xmlconf, lxc_driver-stateDir, vm)
0)
VIR_WARN(Cannot update XML with PID for LXC %s, vm-def-name);
+
+virObjectUnlock(vm);
}
static virLXCMonitorCallbacks monitorCallbacks = {
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/2] Report OOM on VIR_ALLOC failure
On Fri, Mar 22, 2013 at 12:44:55PM +0100, Michal Privoznik wrote:
Currently, our code is plenty of following scheme:
if (VIR_ALLOC(dummyPtr) 0) {
virReportOOMError();
goto cleanup;
}
or something similar. What if we just move the OOM reporting into
VIR_ALLOC? It would have three nice features:
1) sizeof(code base) gets lower. A lot lower.
Yep, makes sense.
2) even for callers which don't follow the schema described
above, there is no harm reporting so serious error in the
logs. No matter that the callee may fall back and return
success.
I think I'd like some analysis of just how many places we
have which call VIR_ALLOC, but don't want to report errors.
Ought to be possible to grep it, or worst case a quick perl
script to analyse thing
At very least we need to be careful with virerror.c and
virlog.c, since both of those allocate memory, and we
don't want them to cause recursion here.
3) Removing virReportOOMError() from the schema does not need to
be done at once, but can be split into several patches. In the
worst case scenario - the error gets reported twice.
If we do this, we must convert everything before any release.
Overwriting errors is something to be avoided, so I don't
want us to make this worse.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] nwfilter: probe for inverted ctdir
Linux netfilter at some point inverted the meaning of the '--ctdir reply'
and newer netfilter implementations now expect '--ctdir original'
instead and vice-versa.
We probe for this netfilter change via a UDP message over loopback and 3
filtering rules applied to INPUT. If the sent byte arrives, the newer
netfilter implementation has been detected.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
v1-v2:
- using virSocketAddrParseIPv4
---
src/nwfilter/nwfilter_ebiptables_driver.c | 121
++
1 file changed, 121 insertions(+)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -27,6 +27,10 @@
#include string.h
#include sys/stat.h
#include fcntl.h
+#include arpa/inet.h
+#include sys/select.h
+#include sys/time.h
+#include unistd.h
#include internal.h
@@ -85,6 +89,12 @@ static char *iptables_cmd_path;
static char *ip6tables_cmd_path;
static char *grep_cmd_path;
+/*
+ * --ctdir original vs. --ctdir reply's meaning was inverted in netfilter
+ * at some point. We probe for it.
+ */
+static bool iptables_ctdir_corrected = false;
+
#define PRINT_ROOT_CHAIN(buf, prefix, ifname) \
snprintf(buf, sizeof(buf), libvirt-%c-%s, prefix, ifname)
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
@@ -1262,6 +1272,9 @@ iptablesEnforceDirection(int directionIn
virNWFilterRuleDefPtr rule,
virBufferPtr buf)
{
+if (iptables_ctdir_corrected)
+directionIn = !directionIn;
+
if (rule-tt != VIR_NWFILTER_RULE_DIRECTION_INOUT)
virBufferAsprintf(buf, -m conntrack --ctdir %s,
(directionIn) ? Original
@@ -4304,6 +4317,111 @@ ebiptablesDriverTestCLITools(void)
return ret;
}
+static void
+ebiptablesDriverProbeCtdir(void)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+static const char cmdline[] =
+ $IPT -%c INPUT %c -i lo -p udp --dport %hu
+ -m state --state ESTABLISHED -j ACCEPT CMD_SEPARATOR
+ $IPT -%c INPUT %c -i lo -p udp --dport %hu
+ -m conntrack --ctdir original -j ACCEPT CMD_SEPARATOR
+ $IPT -%c INPUT %c -i lo -p udp --dport %hu -j DROP;
+/*
+ * Above '--ctdir original' gets this test to receive a message on
+ * 'fixed' netfilter.
+ */
+unsigned short port;
+int ssockfd = -1, csockfd = -1;
+virSocketAddr saddr;
+struct sockaddr_in *serveraddr = saddr.data.inet4;
+fd_set readfds;
+struct timeval timeout = {
+.tv_sec = 0,
+.tv_usec = 1000 * 200,
+};
+int n;
+
+if (virSocketAddrParseIPv4(saddr, 127.0.0.1) 0) {
+VIR_ERROR(_(Could not parse IP address));
+goto cleanup;
+}
+
+if ((ssockfd = socket(AF_INET, SOCK_DGRAM, 0)) 0 ||
+(csockfd = socket(AF_INET, SOCK_DGRAM, 0)) 0) {
+ VIR_ERROR(_(Could not open UDP socket));
+ goto cleanup;
+}
+
+for (port = 0x; port 1024; port--) {
+serveraddr-sin_port = htons(port);
+if (bind(ssockfd, (struct sockaddr *)serveraddr,
+ sizeof(*serveraddr)) == 0)
+break;
+}
+if (port == 1024) {
+VIR_ERROR(_(Could not bind to any UDP socket));
+goto cleanup;
+}
+
+NWFILTER_SET_IPTABLES_SHELLVAR(buf);
+virBufferAsprintf(buf, cmdline,
+ 'I', '1', port,
+ 'I', '2', port,
+ 'I', '3', port);
+
+if (virBufferError(buf)) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (ebiptablesExecCLI(buf, NULL, NULL) 0) {
+VIR_ERROR(_(Could not apply iptables rules));
+goto cleanup_iptables;
+}
+
+if (sendto(csockfd, cmdline, 1, 0, (struct sockaddr *)serveraddr,
+ sizeof(*serveraddr)) 0) {
+VIR_ERROR(_(Could not send to UDP socket));
+goto cleanup_iptables;
+}
+
+FD_ZERO(readfds);
+FD_SET(ssockfd, readfds);
+
+while (true) {
+n = select(ssockfd + 1, readfds, NULL, NULL, timeout);
+if (n 0) {
+if (errno == EINTR)
+continue;
+VIR_ERROR(_(Select failed));
+goto cleanup_iptables;
+}
+if (n == 0) {
+VIR_INFO(Ctdir probing received no data -- 'old' netfilter);
+goto cleanup_iptables;
+}
+VIR_INFO(Ctdir probing received data -- 'fixed' netfilter);
+iptables_ctdir_corrected = true;
+break;
+}
+
+cleanup_iptables:
+virBufferFreeAndReset(buf);
+
+NWFILTER_SET_IPTABLES_SHELLVAR(buf);
+virBufferAsprintf(buf, cmdline,
+ 'D', ' ', port,
+ 'D', ' ', port,
+ 'D', ' ', port);
+ebiptablesExecCLI(buf, NULL, NULL);
+
+cleanup:
+
[libvirt] [PATCH] [TCK] nwfilter: probe for inverted ctdir
Linux netfilter at some point inverted the meaning of the '--ctdir reply'
and newer netfilter implementations now expect '--ctdir original'
instead and vice-versa.
We probe for this netfilter change via an IMCP message over loopback and 3
filtering rules applied to INPUT. If the sent byte arrives, the newer
netfilter implementation has been detected and we convert the strings
in the iptables output to now match that inversion implemented by libvirt.
The downside of this is that probing of libvirt and this test tool are
independent and this test tool will only work correctly for all cases
if used with libvirt probing for 'ctdir inversion' as well.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
scripts/nwfilter/nwfilter2vmtest.sh | 23 +++
1 file changed, 23 insertions(+)
Index: libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
===
--- libvirt-tck.orig/scripts/nwfilter/nwfilter2vmtest.sh
+++ libvirt-tck/scripts/nwfilter/nwfilter2vmtest.sh
@@ -28,6 +28,10 @@ FLAG_LIBVIRT_TEST=$((13))
FLAG_TAP_TEST=$((14))
FLAG_FORCE_CLEAN=$((15))
+# --ctdir original vs. --ctdir reply's meaning was inverted in
+# netfilter at some point. We probe for it.
+IPTABLES_CTRDIR_CORRECTED=0
+
failctr=0
passctr=0
attachfailctr=0
@@ -100,6 +104,15 @@ mktmpdir() {
return 0
}
+probeIptablesCtdir() {
+ iptables -I INPUT 1 -i lo -p icmp -m state --state ESTABLISHED -j ACCEPT
+ iptables -I INPUT 2 -i lo -p icmp -m conntrack --ctdir ORIGINAL -j ACCEPT
+ iptables -I INPUT 3 -i lo -p icmp -j DROP
+ IPTABLES_CTDIR_CORRECTED=$(ping 127.0.0.1 -c 1 | sed -n 's/.*,
\([0-9]*\) received,.*/\1/p')
+ iptables -D INPUT -i lo -p icmp -m state --state ESTABLISHED -j ACCEPT
+ iptables -D INPUT -i lo -p icmp -m conntrack --ctdir ORIGINAL -j ACCEPT
+ iptables -D INPUT -i lo -p icmp -j DROP
+}
checkExpectedOutput() {
xmlfile=$1
@@ -160,6 +173,14 @@ checkExpectedOutput() {
break
fi
+if [ $IPTABLES_CTDIR_CORRECTED -ne 0 ]; then
+ #change --ctdir ORIGINAL to --ctdir REPLY
+ #and--ctdir REPLYto --ctdir ORIGINAL
+ sed -i s/ctdir[ ]*ORIGINAL/ctdir _REPLY/ ${tmpfile}
+ sed -i s/ctdir[ ]*REPLY/ctdir ORIGINAL/ ${tmpfile}
+ sed -i s/ctdir _REPLY/ctdir REPLY/ ${tmpfile}
+fi
+
diff -w ${tmpfile} ${tmpfile2} /dev/null
if [ $? -ne 0 ]; then
@@ -551,6 +572,8 @@ main() {
echo This script will only run on Linux.
fi
exit 1;
+ else
+probeIptablesCtdir
fi
if [ $(($flags $FLAG_TAP_TEST)) -ne 0 ]; then
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Build failed in Jenkins: libvirt-syntax-check #759
On Fri, Mar 22, 2013 at 06:18:48AM -0600, Eric Blake wrote: On 03/21/2013 09:54 PM, Jenkins CI wrote: See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/759/ -- Started by upstream project libvirt-build build number 856 [workspace] $ /bin/sh -xe /tmp/hudson8330114436432330964.sh + make syntax-check GENbracket-spacing-check GFDL_version preprocessor_indentation maint.mk: skipping test sc_preprocessor_indentation: cppi not installed 0.06 preprocessor_indentation Any chance we can install cppi on the Jenkins machine? It isn't packaged in Debian and the upstream tarballs require automake 1.13a which isn't very distributor friendly. Trying to rebuild with older automake gives all parts of fun so I skipped that for now to some for food work pending. prohibit_argmatch_without_use grep: write error grep: write error sed: couldn't write 1 item to stdout: Broken pipe sed: couldn't write 49 items to stdout: Broken pipe 0.58 prohibit_argmatch_without_use There were quite a few of these errors listed; it sounds like it is a bug in the upstream gnulib maint.mk file. Can you help me pinpoint what is going wrong with the syntax check, so we can fix gnulib and make it quieter? Is it just a case of running with SIGPIPE ignored, and hitting a pipeline where the sink of the pipe is exiting early, causing the source to hit EPIPE? I ran this in a look several times and didn't spot the problem. Anything special you want me to try? Cheers, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Build failed in Jenkins: libvirt-syntax-check #759
On Fri, Mar 22, 2013 at 02:10:59PM +0100, Guido Günther wrote: On Fri, Mar 22, 2013 at 06:18:48AM -0600, Eric Blake wrote: On 03/21/2013 09:54 PM, Jenkins CI wrote: See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/759/ -- Started by upstream project libvirt-build build number 856 [workspace] $ /bin/sh -xe /tmp/hudson8330114436432330964.sh + make syntax-check GENbracket-spacing-check GFDL_version preprocessor_indentation maint.mk: skipping test sc_preprocessor_indentation: cppi not installed 0.06 preprocessor_indentation Any chance we can install cppi on the Jenkins machine? It isn't packaged in Debian and the upstream tarballs require automake 1.13a which isn't very distributor friendly. Trying to rebuild with older automake gives all parts of fun so I skipped that for now to some for food work pending. That said building from git worked so it's available now. -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Build failed in Jenkins: libvirt-syntax-check #759
On Fri, Mar 22, 2013 at 02:10:59PM +0100, Guido Günther wrote: [..snip..] prohibit_argmatch_without_use grep: write error grep: write error sed: couldn't write 1 item to stdout: Broken pipe sed: couldn't write 49 items to stdout: Broken pipe 0.58 prohibit_argmatch_without_use There were quite a few of these errors listed; it sounds like it is a bug in the upstream gnulib maint.mk file. Can you help me pinpoint what is going wrong with the syntax check, so we can fix gnulib and make it quieter? Is it just a case of running with SIGPIPE ignored, and hitting a pipeline where the sink of the pipe is exiting early, causing the source to hit EPIPE? I ran this in a look several times and didn't spot the problem. Anything special you want me to try? Following up on this part seperately. This is perfectly reproducible when run from within Jenkins. This is what's used to execute make syntax-check: make syntax-check Which Jenkins simply puts into a script and consumes it's output /bin/sh -xe /tmp/hudson4405079863801767625.sh Note that /bin/sh is dash in this case. Cheers, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Schedule for the next release suggestions
The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, Opinions ? Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Schedule for the next release suggestions
On 22.03.2013 14:36, Daniel Veillard wrote: The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, Opinions ? Daniel Given how hard long we had tried to make the most recent release stable should we prolong the freeze? Or is it something what can be decided operatively once we see how much is upstream broken? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Schedule for the next release suggestions
On Fri, Mar 22, 2013 at 09:36:03PM +0800, Daniel Veillard wrote: The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, I would prefer it if we just stuck to a release on/near April 1st regardless of how many changes have accumulated. IMHO a predictable release date once a month on/near 1st of the month is more important than the amount of code that has been changed. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Schedule for the next release suggestions
On Fri, Mar 22, 2013 at 02:50:22PM +0100, Michal Privoznik wrote: On 22.03.2013 14:36, Daniel Veillard wrote: The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, Opinions ? Daniel Given how hard long we had tried to make the most recent release stable should we prolong the freeze? Or is it something what can be decided operatively once we see how much is upstream broken? In general the 5 days is the 'expected freeze duration assuming the shape is correct' so if we hit something nasty we continue the freeze until problems are fixed. That's actually what happened end of February. Let's say the 5 days is the optimistic version :-) There had been suggestion in the past to branch in git at the freeze time to not penalize continued development, and pull the fixes only in the stable branch for the release. This sounds nice in theory, but this has 2 drawbacks: - a small one which is that the release is no more available on the git master branch - a fairly large one which is that if people keep their focuse on upstream who will spend the time to do the necessary cleanups, and the few who will do will be alone, and it will take more time for them and it is not fun to just do cleanups IMHO cleanup for release really ought to be a wide community effort and that's why I'm still on the side of freezing in master git branch (and anybody can use a local branch to work on their stuff, but community forcus is the freeze at that time). Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Schedule for the next release suggestions
On Fri, Mar 22, 2013 at 02:07:05PM +, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 09:36:03PM +0800, Daniel Veillard wrote: The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, I would prefer it if we just stuck to a release on/near April 1st regardless of how many changes have accumulated. IMHO a predictable release date once a month on/near 1st of the month is more important than the amount of code that has been changed. to release on the 1st we would have to freeze this Monday meaning only 2.5 weeks of development after a freeze for 1.3 which took 1.5 weeks. I was considering the ratio of freeze time vs. open time too. We could try to freeze on the 29 to try to ship on the 5th April one month after the 1.0.3, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Schedule for the next release suggestions
On Fri, Mar 22, 2013 at 10:17:07PM +0800, Daniel Veillard wrote: On Fri, Mar 22, 2013 at 02:07:05PM +, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 09:36:03PM +0800, Daniel Veillard wrote: The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, I would prefer it if we just stuck to a release on/near April 1st regardless of how many changes have accumulated. IMHO a predictable release date once a month on/near 1st of the month is more important than the amount of code that has been changed. to release on the 1st we would have to freeze this Monday meaning only 2.5 weeks of development after a freeze for 1.3 which took 1.5 weeks. I was considering the ratio of freeze time vs. open time too. I don't really think that's a problem myself, it is just to be expected due to the longer than usual 1.3 freeze. People have had just as much time to /write/ their code, all that changed was the time available to /merge/ their code into upstream. I'd like to see is be much stricter about following a monthly release schedule in general. It makes it easier for distro maintainers to accurately plan ahead for when libvirt releases will sync up with their schedules, if they can reliably know that we'll always release on the nearest weekday that follows the 1st of the month. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Move FUSE mount to /var/lib/libvirt/lxc/$NAME.fuse
From: Daniel P. Berrange berra...@redhat.com
Instead of using /var/lib/libvirt/lxc/$NAME for the FUSE
filesystem, use /var/lib/libvirt/lxc/$NAME.fuse. This allows
room for other temporary mounts in the same directory
---
src/lxc/lxc_container.c | 2 +-
src/lxc/lxc_fuse.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d09791..bf17a38 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -604,7 +604,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
char *meminfo_path = NULL;
if ((ret = virAsprintf(meminfo_path,
- %s/%s/%s/meminfo,
+ %s/%s/%s.fuse/meminfo,
srcprefix ? srcprefix : , LXC_STATE_DIR,
def-name)) 0)
return ret;
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index c4be58e..fbd0d56 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -291,7 +291,7 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def)
if (virMutexInit(fuse-lock) 0)
goto cleanup2;
-if (virAsprintf(fuse-mountpoint, %s/%s/, LXC_STATE_DIR,
+if (virAsprintf(fuse-mountpoint, %s/%s.fuse/, LXC_STATE_DIR,
def-name) 0) {
virReportOOMError();
goto cleanup1;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Mount temporary devpts on /var/lib/libvirt/lxc/$NAME.devpts
From: Daniel P. Berrange berra...@redhat.com
Currently the lxc controller sets up the devpts instance on
$rootfsdef-src, but this only works if $rootfsdef is using
type=mount. To support type=block or type=file for the root
filesystem, we must use /var/lib/libvirt/lxc/$NAME.devpts
for the temporary devpts mount in the controller
---
src/lxc/lxc_container.c | 43 +--
src/lxc/lxc_controller.c | 13 -
2 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index bf17a38..be9bc6c 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -627,15 +627,17 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def
ATTRIBUTE_UNUSED,
}
#endif
-static int lxcContainerMountFSDevPTS(virDomainFSDefPtr root)
+static int lxcContainerMountFSDevPTS(virDomainDefPtr def,
+ const char *srcprefix)
{
-char *devpts = NULL;
-int rc = -1;
+int ret;
+char *path = NULL;
-if (virAsprintf(devpts, /.oldroot%s/dev/pts, root-src) 0) {
-virReportOOMError();
-goto cleanup;
-}
+if ((ret = virAsprintf(path,
+ %s/%s/%s.devpts,
+ srcprefix ? srcprefix : , LXC_STATE_DIR,
+ def-name)) 0)
+return ret;
if (virFileMakePath(/dev/pts) 0) {
virReportSystemError(errno, %s,
@@ -643,19 +645,20 @@ static int lxcContainerMountFSDevPTS(virDomainFSDefPtr
root)
goto cleanup;
}
-VIR_DEBUG(Trying to move %s to %s, devpts, /dev/pts);
-if ((rc = mount(devpts, /dev/pts, NULL, MS_MOVE, NULL)) 0) {
-virReportSystemError(errno, %s,
- _(Failed to mount /dev/pts in container));
+VIR_DEBUG(Trying to move %s to /dev/pts, path);
+
+if ((ret = mount(path, /dev/pts,
+ NULL, MS_MOVE, NULL)) 0) {
+virReportSystemError(errno,
+ _(Failed to mount %s on /dev/pts),
+ path);
goto cleanup;
}
-rc = 0;
-
- cleanup:
-VIR_FREE(devpts);
+cleanup:
+VIR_FREE(path);
-return rc;
+return ret;
}
static int lxcContainerPopulateDevices(char **ttyPaths, size_t nttyPaths)
@@ -1961,7 +1964,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr
vmDef,
goto cleanup;
/* Mounts /dev/pts */
-if (lxcContainerMountFSDevPTS(root) 0)
+if (lxcContainerMountFSDevPTS(vmDef, /.oldroot) 0)
goto cleanup;
/* Populates device nodes in /dev/ */
@@ -2204,7 +2207,11 @@ static int lxcContainerChild(void *data)
if (argv-nttyPaths) {
if (root) {
-if (virAsprintf(ttyPath, %s%s, root-src, argv-ttyPaths[0])
0) {
+const char *tty = argv-ttyPaths[0];
+if (STRPREFIX(tty, /dev/pts/))
+tty += strlen(/dev/pts/);
+if (virAsprintf(ttyPath, %s/%s.devpts/%s,
+LXC_STATE_DIR, vmDef-name, tty) 0) {
virReportOOMError();
goto cleanup;
}
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index bb369e2..1d1443c 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1219,15 +1219,10 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl)
mount_options = virSecurityManagerGetMountOptions(ctrl-securityManager,
ctrl-def);
-if (!virFileExists(root-src)) {
-virReportSystemError(errno,
- _(root source %s does not exist),
- root-src);
-goto cleanup;
-}
-
-if (virAsprintf(devpts, %s/dev/pts, root-src) 0 ||
-virAsprintf(ctrl-devptmx, %s/dev/pts/ptmx, root-src) 0) {
+if (virAsprintf(devpts, %s/%s.devpts,
+LXC_STATE_DIR, ctrl-def-name) 0 ||
+virAsprintf(ctrl-devptmx, %s/%s.devpts/ptmx,
+LXC_STATE_DIR, ctrl-def-name) 0) {
virReportOOMError();
goto cleanup;
}
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Ensure root filesystem is mounted if a file/block mount.
From: Daniel P. Berrange berra...@redhat.com
For a root filesystem with type=file or type=block, the LXC
container was forgetting to actually mount it, before doing
the pivot root step.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_container.c | 52 +
1 file changed, 52 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index be9bc6c..002dba1 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -105,6 +105,9 @@ struct __lxc_child_argv {
int handshakefd;
};
+static int lxcContainerMountFSBlock(virDomainFSDefPtr fs,
+const char *srcprefix);
+
/*
* reboot(LINUX_REBOOT_CMD_CAD_ON) will return -EINVAL
@@ -406,6 +409,51 @@ static int lxcContainerChildMountSort(const void *a, const
void *b)
# define MS_SLAVE(119)
#endif
+static int lxcContainerPrepareRoot(virDomainDefPtr def,
+ virDomainFSDefPtr root)
+{
+char *dst;
+char *tmp;
+
+if (root-type == VIR_DOMAIN_FS_TYPE_MOUNT)
+return 0;
+
+if (root-type == VIR_DOMAIN_FS_TYPE_FILE) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unexpected root filesystem without loop device));
+return -1;
+}
+
+if (root-type != VIR_DOMAIN_FS_TYPE_BLOCK) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported root filesystem type %s),
+ virDomainFSTypeToString(root-type));
+return -1;
+}
+
+if (virAsprintf(dst, %s/%s.root,
+LXC_STATE_DIR, def-name) 0) {
+virReportOOMError();
+return -1;
+}
+
+tmp = root-dst;
+root-dst = dst;
+
+if (lxcContainerMountFSBlock(root, ) 0) {
+root-dst = tmp;
+VIR_FREE(dst);
+return -1;
+}
+
+root-dst = tmp;
+root-type = VIR_DOMAIN_FS_TYPE_MOUNT;
+VIR_FREE(root-src);
+root-src = dst;
+
+return 0;
+}
+
static int lxcContainerPivotRoot(virDomainFSDefPtr root)
{
int ret;
@@ -1926,6 +1974,10 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr
vmDef,
if (lxcContainerIdentifyCGroups(mounts, nmounts, cgroupRoot) 0)
goto cleanup;
+/* Ensure the root filesystem is mounted */
+if (lxcContainerPrepareRoot(vmDef, root) 0)
+goto cleanup;
+
/* Gives us a private root, leaving all parent OS mounts on /.oldroot */
if (lxcContainerPivotRoot(root) 0)
goto cleanup;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Refactoring of cgroups usage
Currently libvirtd creates its basic cgroup hierarchy when it starts up. This is bad because if people mount cgroups after libvirtd it is running it doesn't detect this. The second issue is that the driver code re-creates the virCgroupPtr instance for a domain over over again, which is inefficient and bloats the code. Finally, if the driver is configured to only use a couple of the cgroups controllers, we are still creating the directories under all of them -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] Rename virCgroupMounted to virCgroupHasController make it more robust
From: Daniel P. Berrange berra...@redhat.com
The virCgroupMounted method is badly named, since a controller can be
mounted, but disabled in the current object. Rename the method to be
virCgroupHasController. Also make it tolerant to a NULL virCgroupPtr
and out-of-range controller index, to avoid duplication of these
checks in all callers
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 2 +-
src/lxc/lxc_driver.c | 12 +---
src/lxc/lxc_process.c| 10 +-
src/qemu/qemu_cgroup.c | 14 +-
src/util/vircgroup.c | 13 +
src/util/vircgroup.h | 2 +-
6 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f75d681..f23f80d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -,7 +,7 @@ virCgroupGetMemSwapUsage;
virCgroupKill;
virCgroupKillPainfully;
virCgroupKillRecursive;
-virCgroupMounted;
+virCgroupHasController;
virCgroupMoveTask;
virCgroupPathOfController;
virCgroupRemove;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 989c2f3..6737f13 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1641,17 +1641,7 @@ cleanup:
static bool lxcCgroupControllerActive(virLXCDriverPtr driver,
int controller)
{
-if (driver-cgroup == NULL)
-return false;
-if (controller 0 || controller = VIR_CGROUP_CONTROLLER_LAST)
-return false;
-if (!virCgroupMounted(driver-cgroup, controller))
-return false;
-#if 0
-if (driver-cgroupControllers (1 controller))
-return true;
-#endif
-return true;
+return virCgroupHasController(driver-cgroup, controller);
}
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 670a032..8c8778a 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1006,20 +1006,20 @@ int virLXCProcessStart(virConnectPtr conn,
return -1;
}
-if (!virCgroupMounted(lxc_driver-cgroup,
+if (!virCgroupHasController(lxc_driver-cgroup,
VIR_CGROUP_CONTROLLER_CPUACCT)) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Unable to find 'cpuacct' cgroups controller mount));
return -1;
}
-if (!virCgroupMounted(lxc_driver-cgroup,
- VIR_CGROUP_CONTROLLER_DEVICES)) {
+if (!virCgroupHasController(lxc_driver-cgroup,
+VIR_CGROUP_CONTROLLER_DEVICES)) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Unable to find 'devices' cgroups controller mount));
return -1;
}
-if (!virCgroupMounted(lxc_driver-cgroup,
- VIR_CGROUP_CONTROLLER_MEMORY)) {
+if (!virCgroupHasController(lxc_driver-cgroup,
+VIR_CGROUP_CONTROLLER_MEMORY)) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Unable to find 'memory' cgroups controller mount));
return -1;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2cdc2b7..5aa9416 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -48,19 +48,7 @@ static const char *const defaultDeviceACL[] = {
bool qemuCgroupControllerActive(virQEMUDriverPtr driver,
int controller)
{
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-bool ret = false;
-
-if (driver-cgroup == NULL)
-goto cleanup;
-if (controller 0 || controller = VIR_CGROUP_CONTROLLER_LAST)
-goto cleanup;
-if (!virCgroupMounted(driver-cgroup, controller))
-goto cleanup;
-
-cleanup:
-virObjectUnref(cfg);
-return ret;
+return virCgroupHasController(driver-cgroup, controller);
}
static int
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 0d6db58..ccdb1dd 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -94,15 +94,20 @@ void virCgroupFree(virCgroupPtr *group)
}
/**
- * virCgroupMounted: query whether a cgroup subsystem is mounted or not
+ * virCgroupHasController: query whether a cgroup controller is present
*
- * @cgroup: The group structure to be queried
+ * @cgroup: The group structure to be queried, or NULL
* @controller: cgroup subsystem id
*
- * Returns true if a cgroup is subsystem is mounted.
+ * Returns true if a cgroup controller is mounted and is associated
+ * with this cgroup object.
*/
-bool virCgroupMounted(virCgroupPtr cgroup, int controller)
+bool virCgroupHasController(virCgroupPtr cgroup, int controller)
{
+if (!cgroup)
+return false;
+if (controller 0 || controller = VIR_CGROUP_CONTROLLER_LAST)
+return false;
return cgroup-controllers[controller].mountPoint != NULL;
}
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 725d2d0..4c1134d 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@
[libvirt] [PATCH 1/5] Rename virCgroupGetAppRoot to virCgroupForSelf
From: Daniel P. Berrange berra...@redhat.com
The virCgroupGetAppRoot is not clear in its meaning. Change
to virCgroupForSelf to highlight that this returns the
cgroup config for the caller's process
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 2 +-
src/lxc/lxc_cgroup.c | 2 +-
src/util/vircgroup.c | 9 ++---
src/util/vircgroup.h | 2 +-
4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 21bc615..f75d681 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1090,9 +1090,9 @@ virCgroupDenyDevicePath;
virCgroupForDomain;
virCgroupForDriver;
virCgroupForEmulator;
+virCgroupForSelf;
virCgroupForVcpu;
virCgroupFree;
-virCgroupGetAppRoot;
virCgroupGetBlkioWeight;
virCgroupGetCpuacctPercpuUsage;
virCgroupGetCpuacctStat;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index df468da..33c305a 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -293,7 +293,7 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo)
int ret;
virCgroupPtr cgroup;
-ret = virCgroupGetAppRoot(cgroup);
+ret = virCgroupForSelf(cgroup);
if (ret 0) {
virReportSystemError(-ret, %s,
_(Unable to get cgroup for container));
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 6998f13..266cecb 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -967,19 +967,22 @@ int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED,
#endif
/**
-* virCgroupGetAppRoot:
+* virCgroupForSelf:
*
* @group: Pointer to returned virCgroupPtr
*
+* Obtain a cgroup representing the config of the
+* current process
+*
* Returns 0 on success
*/
#if defined HAVE_MNTENT_H defined HAVE_GETMNTENT_R
-int virCgroupGetAppRoot(virCgroupPtr *group)
+int virCgroupForSelf(virCgroupPtr *group)
{
return virCgroupNew(/, group);
}
#else
-int virCgroupGetAppRoot(virCgroupPtr *group ATTRIBUTE_UNUSED)
+int virCgroupForSelf(virCgroupPtr *group ATTRIBUTE_UNUSED)
{
return -ENXIO;
}
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index ea42fa2..45a2006 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -49,7 +49,7 @@ int virCgroupForDriver(const char *name,
bool privileged,
bool create);
-int virCgroupGetAppRoot(virCgroupPtr *group);
+int virCgroupForSelf(virCgroupPtr *group);
int virCgroupForDomain(virCgroupPtr driver,
const char *name,
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] Store a virCgroupPtr instance in virLXCDomainObjPrivatePtr
From: Daniel P. Berrange berra...@redhat.com
Instead of calling virCgroupForDomain every time we need
the virCgrouPtr instance, just do it once at Vm startup
and cache a reference to the object in virLXCDomainObjPrivatePtr
until shutdown of the VM. Removing the virCgroupPtr from
the LXC driver state also means we don't have stale mount
info, if someone mounts the cgroups filesystem after libvirtd
has been started
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_cgroup.c | 2 +-
src/lxc/lxc_cgroup.h | 1 +
src/lxc/lxc_conf.h| 3 -
src/lxc/lxc_domain.c | 2 +
src/lxc/lxc_domain.h | 3 +
src/lxc/lxc_driver.c | 354 ++
src/lxc/lxc_process.c | 46 ---
7 files changed, 135 insertions(+), 276 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 545acaa..0a2c795 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -530,7 +530,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
int ret = -1;
int rc;
-rc = virCgroupForDriver(lxc, driver, true, false, 0);
+rc = virCgroupForDriver(lxc, driver, true, false, -1);
if (rc != 0) {
virReportSystemError(-rc, %s,
_(Unable to get cgroup for driver));
diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h
index 942e0fc..46dc3bb 100644
--- a/src/lxc/lxc_cgroup.h
+++ b/src/lxc/lxc_cgroup.h
@@ -22,6 +22,7 @@
#ifndef __VIR_LXC_CGROUP_H__
# define __VIR_LXC_CGROUP_H__
+# include vircgroup.h
# include domain_conf.h
# include lxc_fuse.h
# include virusb.h
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index b46dc32..dbe13a5 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -32,7 +32,6 @@
# include domain_event.h
# include capabilities.h
# include virthread.h
-# include vircgroup.h
# include security/security_manager.h
# include configmake.h
# include virusb.h
@@ -53,8 +52,6 @@ struct _virLXCDriver {
virCapsPtr caps;
virDomainXMLConfPtr xmlconf;
-virCgroupPtr cgroup;
-
size_t nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index 08cf8f6..1364e8e 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -43,6 +43,8 @@ static void virLXCDomainObjPrivateFree(void *data)
{
virLXCDomainObjPrivatePtr priv = data;
+virCgroupFree(priv-cgroup);
+
VIR_FREE(priv);
}
diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
index 007ea84..1bc8ce5 100644
--- a/src/lxc/lxc_domain.h
+++ b/src/lxc/lxc_domain.h
@@ -23,6 +23,7 @@
#ifndef __LXC_DOMAIN_H__
# define __LXC_DOMAIN_H__
+# include vircgroup.h
# include lxc_conf.h
# include lxc_monitor.h
@@ -36,6 +37,8 @@ struct _virLXCDomainObjPrivate {
bool wantReboot;
pid_t initpid;
+
+virCgroupPtr cgroup;
};
extern virDomainXMLPrivateDataCallbacks virLXCDriverPrivateDataCallbacks;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6737f13..91f14cd 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -527,8 +527,8 @@ static int lxcDomainGetInfo(virDomainPtr dom,
{
virLXCDriverPtr driver = dom-conn-privateData;
virDomainObjPtr vm;
-virCgroupPtr cgroup = NULL;
int ret = -1, rc;
+virLXCDomainObjPrivatePtr priv;
lxcDriverLock(driver);
vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
@@ -541,24 +541,20 @@ static int lxcDomainGetInfo(virDomainPtr dom,
goto cleanup;
}
+priv = vm-privateData;
+
info-state = virDomainObjGetState(vm, NULL);
-if (!virDomainObjIsActive(vm) || driver-cgroup == NULL) {
+if (!virDomainObjIsActive(vm)) {
info-cpuTime = 0;
info-memory = vm-def-mem.cur_balloon;
} else {
-if (virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0) !=
0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Unable to get cgroup for %s), vm-def-name);
-goto cleanup;
-}
-
-if (virCgroupGetCpuacctUsage(cgroup, (info-cpuTime)) 0) {
+if (virCgroupGetCpuacctUsage(priv-cgroup, (info-cpuTime)) 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
%s, _(Cannot read cputime for domain));
goto cleanup;
}
-if ((rc = virCgroupGetMemoryUsage(cgroup, (info-memory))) 0) {
+if ((rc = virCgroupGetMemoryUsage(priv-cgroup, (info-memory))) 0)
{
virReportError(VIR_ERR_OPERATION_FAILED,
%s, _(Cannot read memory usage for domain));
if (rc == -ENOENT) {
@@ -576,8 +572,6 @@ static int lxcDomainGetInfo(virDomainPtr dom,
cleanup:
lxcDriverUnlock(driver);
-if (cgroup)
-virCgroupFree(cgroup);
if (vm)
virObjectUnlock(vm);
return ret;
@@ -708,8 +702,8 @@ cleanup:
static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
[libvirt] [PATCH 2/5] Don't create dirs in cgroup controllers we don't want to use
From: Daniel P. Berrange berra...@redhat.com
Currently when getting an instance of virCgroupPtr we will
create the path in all cgroup controllers. Only at the virt
driver layer are we attempting to filter controllers. This
is bad because the mere act of creating the dirs in the
controllers can have a functional impact on the kernel,
particularly for performance.
Update the virCgroupForDriver() method to accept a bitmask
of controllers to use. Only create dirs in the controllers
that are requested. When creating cgroups for domains,
respect the active controller list from the parent cgroup
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_cgroup.c | 2 +-
src/lxc/lxc_driver.c | 2 +-
src/qemu/qemu_cgroup.c | 27 +++-
src/qemu/qemu_conf.c | 16 +
src/qemu/qemu_driver.c | 3 +-
src/util/vircgroup.c | 181 +++--
src/util/vircgroup.h | 6 +-
7 files changed, 132 insertions(+), 105 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 33c305a..545acaa 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -530,7 +530,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
int ret = -1;
int rc;
-rc = virCgroupForDriver(lxc, driver, 1, 0);
+rc = virCgroupForDriver(lxc, driver, true, false, 0);
if (rc != 0) {
virReportSystemError(-rc, %s,
_(Unable to get cgroup for driver));
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 8603078..989c2f3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1460,7 +1460,7 @@ static int lxcStartup(bool privileged,
lxc_driver-log_libvirtd = 0; /* by default log to container logfile */
lxc_driver-have_netns = lxcCheckNetNsSupport();
-rc = virCgroupForDriver(lxc, lxc_driver-cgroup, privileged, 1);
+rc = virCgroupForDriver(lxc, lxc_driver-cgroup, privileged, true, -1);
if (rc 0) {
char buf[1024] ATTRIBUTE_UNUSED;
VIR_DEBUG(Unable to create cgroup for LXC driver: %s,
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index c9b4ca2..2cdc2b7 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -57,8 +57,6 @@ bool qemuCgroupControllerActive(virQEMUDriverPtr driver,
goto cleanup;
if (!virCgroupMounted(driver-cgroup, controller))
goto cleanup;
-if (cfg-cgroupControllers (1 controller))
-ret = true;
cleanup:
virObjectUnref(cfg);
@@ -668,7 +666,7 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver,
virDomainDefPtr def = vm-def;
unsigned long long period = vm-def-cputune.emulator_period;
long long quota = vm-def-cputune.emulator_quota;
-int rc, i;
+int rc;
if ((period || quota)
(!driver-cgroup ||
@@ -697,22 +695,13 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver,
goto cleanup;
}
-for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
-if (i != VIR_CGROUP_CONTROLLER_CPU
-i != VIR_CGROUP_CONTROLLER_CPUACCT
-i != VIR_CGROUP_CONTROLLER_CPUSET)
-continue;
-
-if (!qemuCgroupControllerActive(driver, i))
-continue;
-rc = virCgroupMoveTask(cgroup, cgroup_emulator, i);
-if (rc 0) {
-virReportSystemError(-rc,
- _(Unable to move tasks from domain cgroup to
- emulator cgroup in controller %d for %s),
- i, vm-def-name);
-goto cleanup;
-}
+rc = virCgroupMoveTask(cgroup, cgroup_emulator);
+if (rc 0) {
+virReportSystemError(-rc,
+ _(Unable to move tasks from domain cgroup to
+ emulator cgroup for %s),
+ vm-def-name);
+goto cleanup;
}
if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c2e2e10..5a3dde0 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -134,14 +134,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
privileged)
}
cfg-dynamicOwnership = privileged;
-cfg-cgroupControllers =
-(1 VIR_CGROUP_CONTROLLER_CPU) |
-(1 VIR_CGROUP_CONTROLLER_DEVICES) |
-(1 VIR_CGROUP_CONTROLLER_MEMORY) |
-(1 VIR_CGROUP_CONTROLLER_BLKIO) |
-(1 VIR_CGROUP_CONTROLLER_CPUSET) |
-(1 VIR_CGROUP_CONTROLLER_CPUACCT);
-
+cfg-cgroupControllers = -1; /* -1 == auto-detect */
if (privileged) {
if (virAsprintf(cfg-logDir,
@@ -454,6 +447,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
p = virConfGetValue(conf, cgroup_controllers);
CHECK_TYPE(cgroup_controllers, VIR_CONF_LIST);
if (p) {
+cfg-cgroupControllers = 0;
virConfValuePtr pp;
for (i = 0, pp = p-list; pp; ++i, pp =
Re: [libvirt] Schedule for the next release suggestions
On Fri, Mar 22, 2013 at 02:23:34PM +, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 10:17:07PM +0800, Daniel Veillard wrote: On Fri, Mar 22, 2013 at 02:07:05PM +, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 09:36:03PM +0800, Daniel Veillard wrote: The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, I would prefer it if we just stuck to a release on/near April 1st regardless of how many changes have accumulated. IMHO a predictable release date once a month on/near 1st of the month is more important than the amount of code that has been changed. to release on the 1st we would have to freeze this Monday meaning only 2.5 weeks of development after a freeze for 1.3 which took 1.5 weeks. I was considering the ratio of freeze time vs. open time too. I don't really think that's a problem myself, it is just to be expected due to the longer than usual 1.3 freeze. People have had just as much time to /write/ their code, all that changed was the time available to /merge/ their code into upstream. I'd like to see is be much stricter about following a monthly release schedule in general. It makes it easier for distro maintainers to accurately plan ahead for when libvirt releases will sync up with their schedules, if they can reliably know that we'll always release on the nearest weekday that follows the 1st of the month. So that mean freezing on monday, I can do that ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Mount temporary devpts on /var/lib/libvirt/lxc/$NAME.devpts
On 22.03.2013 15:24, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently the lxc controller sets up the devpts instance on $rootfsdef-src, but this only works if $rootfsdef is using type=mount. To support type=block or type=file for the root filesystem, we must use /var/lib/libvirt/lxc/$NAME.devpts for the temporary devpts mount in the controller --- src/lxc/lxc_container.c | 43 +-- src/lxc/lxc_controller.c | 13 - 2 files changed, 29 insertions(+), 27 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Move FUSE mount to /var/lib/libvirt/lxc/$NAME.fuse
On 22.03.2013 15:24, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Instead of using /var/lib/libvirt/lxc/$NAME for the FUSE
filesystem, use /var/lib/libvirt/lxc/$NAME.fuse. This allows
room for other temporary mounts in the same directory
---
src/lxc/lxc_container.c | 2 +-
src/lxc/lxc_fuse.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d09791..bf17a38 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -604,7 +604,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
char *meminfo_path = NULL;
if ((ret = virAsprintf(meminfo_path,
- %s/%s/%s/meminfo,
+ %s/%s/%s.fuse/meminfo,
srcprefix ? srcprefix : , LXC_STATE_DIR,
def-name)) 0)
return ret;
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index c4be58e..fbd0d56 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -291,7 +291,7 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def)
if (virMutexInit(fuse-lock) 0)
goto cleanup2;
-if (virAsprintf(fuse-mountpoint, %s/%s/, LXC_STATE_DIR,
+if (virAsprintf(fuse-mountpoint, %s/%s.fuse/, LXC_STATE_DIR,
def-name) 0) {
virReportOOMError();
goto cleanup1;
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ensure root filesystem is mounted if a file/block mount.
On 22.03.2013 15:24, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
For a root filesystem with type=file or type=block, the LXC
container was forgetting to actually mount it, before doing
the pivot root step.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_container.c | 52
+
1 file changed, 52 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index be9bc6c..002dba1 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -105,6 +105,9 @@ struct __lxc_child_argv {
int handshakefd;
};
+static int lxcContainerMountFSBlock(virDomainFSDefPtr fs,
+const char *srcprefix);
+
1: ^^^
/*
* reboot(LINUX_REBOOT_CMD_CAD_ON) will return -EINVAL
@@ -406,6 +409,51 @@ static int lxcContainerChildMountSort(const void *a,
const void *b)
# define MS_SLAVE(119)
#endif
+static int lxcContainerPrepareRoot(virDomainDefPtr def,
+ virDomainFSDefPtr root)
+{
+char *dst;
+char *tmp;
+
+if (root-type == VIR_DOMAIN_FS_TYPE_MOUNT)
+return 0;
+
+if (root-type == VIR_DOMAIN_FS_TYPE_FILE) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unexpected root filesystem without loop device));
+return -1;
+}
+
+if (root-type != VIR_DOMAIN_FS_TYPE_BLOCK) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unsupported root filesystem type %s),
+ virDomainFSTypeToString(root-type));
+return -1;
+}
+
+if (virAsprintf(dst, %s/%s.root,
+LXC_STATE_DIR, def-name) 0) {
+virReportOOMError();
+return -1;
+}
+
+tmp = root-dst;
+root-dst = dst;
+
+if (lxcContainerMountFSBlock(root, ) 0) {
+root-dst = tmp;
+VIR_FREE(dst);
+return -1;
+}
+
+root-dst = tmp;
+root-type = VIR_DOMAIN_FS_TYPE_MOUNT;
+VIR_FREE(root-src);
+root-src = dst;
+
+return 0;
+}
+
Any chance you can move this after the lxcContainerMountFSBlock() and
hence avoid [1]? Not a show stopper though. ACK.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Schedule for the next release suggestions
On 22.03.2013 15:38, Daniel Veillard wrote: On Fri, Mar 22, 2013 at 02:23:34PM +, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 10:17:07PM +0800, Daniel Veillard wrote: On Fri, Mar 22, 2013 at 02:07:05PM +, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 09:36:03PM +0800, Daniel Veillard wrote: The 1.0.3 release was on the 5th March, and right now we have accumulated 'only' 150 commits since the release. Based on this I would suggest to wait a couple of weeks before to enter a freeze for the following release. This mean we drift from the usual end of month, but the ratio of freeze/devel will remain more or less constant as well as the expected size of change in the new release. So if this is fine I would suggest to enter freeze for 1.0.4 on the 5th of April for a release around the 12. Unless there is a reason to push a release earlier, I would prefer it if we just stuck to a release on/near April 1st regardless of how many changes have accumulated. IMHO a predictable release date once a month on/near 1st of the month is more important than the amount of code that has been changed. to release on the 1st we would have to freeze this Monday meaning only 2.5 weeks of development after a freeze for 1.3 which took 1.5 weeks. I was considering the ratio of freeze time vs. open time too. I don't really think that's a problem myself, it is just to be expected due to the longer than usual 1.3 freeze. People have had just as much time to /write/ their code, all that changed was the time available to /merge/ their code into upstream. I'd like to see is be much stricter about following a monthly release schedule in general. It makes it easier for distro maintainers to accurately plan ahead for when libvirt releases will sync up with their schedules, if they can reliably know that we'll always release on the nearest weekday that follows the 1st of the month. So that mean freezing on monday, I can do that ! Daniel That effectively cuts a day for us, leaving us with only four days of freeze since on 1st April it's Easter Monday = public holiday in most countries (at least I've taken look at UK and Czech Republic). But I can live with that since the current status of the HEAD looks good. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Move FUSE mount to /var/lib/libvirt/lxc/$NAME.fuse
On 03/22/2013 10:24 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Instead of using /var/lib/libvirt/lxc/$NAME for the FUSE
filesystem, use /var/lib/libvirt/lxc/$NAME.fuse. This allows
room for other temporary mounts in the same directory
---
src/lxc/lxc_container.c | 2 +-
src/lxc/lxc_fuse.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d09791..bf17a38 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -604,7 +604,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
char *meminfo_path = NULL;
if ((ret = virAsprintf(meminfo_path,
- %s/%s/%s/meminfo,
+ %s/%s/%s.fuse/meminfo,
srcprefix ? srcprefix : , LXC_STATE_DIR,
def-name)) 0)
return ret;
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index c4be58e..fbd0d56 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -291,7 +291,7 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def)
if (virMutexInit(fuse-lock) 0)
goto cleanup2;
-if (virAsprintf(fuse-mountpoint, %s/%s/, LXC_STATE_DIR,
+if (virAsprintf(fuse-mountpoint, %s/%s.fuse/, LXC_STATE_DIR,
def-name) 0) {
virReportOOMError();
goto cleanup1;
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc: Prevent shutting down the host
On Thu, Mar 21, 2013 at 04:10:45PM +0100, Martin Kletzander wrote: When the container has the same '/dev' mount as host (no chroot), calling domainShutdown(WithFlags) shouldn't shutdown the host it is running on. Signed-off-by: Martin Kletzander mklet...@redhat.com --- This is also valid for 1.0.[23]-maint branches, so in case this gets ACK'd I'll either send a follow-up for those or push it there as well (if the ACK says so). src/lxc/lxc_driver.c | 45 - 1 file changed, 28 insertions(+), 17 deletions(-) ACK, as a temporary measure. I think we need to make sure that /dev is always private for all LXC containers. That can wait for a more general refactoring of LXC filesystem setup though. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Mount temporary devpts on /var/lib/libvirt/lxc/$NAME.devpts
On 03/22/2013 10:24 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently the lxc controller sets up the devpts instance on $rootfsdef-src, but this only works if $rootfsdef is using type=mount. To support type=block or type=file for the root filesystem, we must use /var/lib/libvirt/lxc/$NAME.devpts for the temporary devpts mount in the controller ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ensure root filesystem is mounted if a file/block mount.
On 03/22/2013 10:24 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com For a root filesystem with type=file or type=block, the LXC container was forgetting to actually mount it, before doing the pivot root step. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix free of uninitialized value in LXC numad setup
On 03/22/2013 07:45 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The 'nodeset' variable was never initialized, causing a later
VIR_FREE(nodeset) to free uninitialized memory.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_controller.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Pushed under trivial rule
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 37e3ce9..bb369e2 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -517,7 +517,7 @@ static int
virLXCControllerGetNumadAdvice(virLXCControllerPtr ctrl,
virBitmapPtr *mask)
{
virBitmapPtr nodemask = NULL;
-char *nodeset;
+char *nodeset = NULL;
int ret = -1;
/* Get the advisory nodeset from numad if 'placement' of
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 1/2] viralloc: Report OOM error on failure
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
In nearly all cases of calling VIR_ALLOC*, VIR_REALLOC_N,
VIR_EXPAND_N, VIR_RESIZE_N, VIR_*_ELEMENT etc. we want to
report OOM error, so our source code base is full of:
if (VIR_ALLOC(somePtr) 0) {
virReportOOMError();
goto cleanup;
}
or similar. Moreover, for those few cases where we don't want to
report OOM error (e.g. virReportOOMError() itself) a new
VIR_ALLOC_NOOOM macro is being introduced.
---
[...]
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7be7f82..30ffe15 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -46,7 +46,7 @@
/* Don't call these directly - use the macros below */
-int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK
+int virAlloc(void *ptrptr, size_t size, bool report) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
@@ -76,13 +76,16 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* VIR_ALLOC:
* @ptr: pointer to hold address of allocated memory
*
- * Allocate sizeof(*ptr) bytes of memory and store
- * the address of allocated memory in 'ptr'. Fill the
- * newly allocated memory with zeros.
+ * Allocate sizeof(*ptr) bytes of memory and store the
+ * address of allocated memory in 'ptr'. Fill the newly
+ * allocated memory with zeros. If there's a failure,
+ * OOM error is reported. The VIR_ALLOC_NOOOM macro
+ * behaves the same except the OOM error reporting.
*
* Returns -1 on failure, 0 on success
*/
-# define VIR_ALLOC(ptr) virAlloc((ptr), sizeof(*(ptr)))
+# define VIR_ALLOC(ptr) virAlloc((ptr), sizeof(*(ptr)), true)
+# define VIR_ALLOC_NOOOM(ptr) virAlloc((ptr), sizeof(*(ptr)), true)
should be false in the 2nd case...
/**
* VIR_ALLOC_N:
diff --git a/src/util/virerror.c b/src/util/virerror.c
index c30642a..c033129 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -204,7 +204,7 @@ virLastErrorObject(void)
virErrorPtr err;
err = virThreadLocalGet(virLastErr);
if (!err) {
-if (VIR_ALLOC(err) 0)
+if (VIR_ALLOC_NOOOM(err) 0)
return NULL;
if (virThreadLocalSet(virLastErr, err) 0)
VIR_FREE(err);
ACK with nit fixed.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Problems with filesystem type='block'
On Fri, Mar 22, 2013 at 11:59:56AM +, Daniel P. Berrange wrote: On Thu, Mar 21, 2013 at 11:50:40PM -0500, Doug Goldstein wrote: So still trying to figure out what I'm doing wrong with LXC because I just can't get any joy. So I'll go one issue at a time. The following VM definition: domain type='lxc' nametestdeb/name uuiddf03b2ce-725a-42e2-39e4-d646be8facb3/uuid memory unit='KiB'332768/memory currentMemory unit='KiB'332768/currentMemory vcpu placement='static'1/vcpu os type arch='x86_64'exe/type init/sbin/init/init /os clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashdestroy/on_crash devices emulator/usr/libexec/libvirt_lxc/emulator filesystem type='block' accessmode='passthrough' source dev='/dev/mapper/vms-testdeb'/ target dir='/'/ /filesystem So, it seems I only tested 'type=block with non-/ filesystem mounts. I can confirm it is broken for target dir=//. The fix is not exactly trivial, so might take me a while Ok, I've pushed 5 changes which fix this setup for me, so hopefully it will work for you too. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 2/2] util: Don't report OOM twice
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
Adapt code under src/util/ to fact, that VIR_ALLOC* now reports OOM
error. There is no need to report it twice now.
---
src/util/iohelper.c | 4 +---
src/util/virauthconfig.c | 8 ++--
src/util/vircommand.c| 13 +++--
src/util/virconf.c | 10 ++
src/util/virdnsmasq.c| 19 +++
src/util/vireventpoll.c | 4 +---
src/util/virfile.c | 4 +---
src/util/virhash.c | 10 ++
src/util/virkeyfile.c| 4 +---
src/util/virlockspace.c | 11 +++
src/util/virnetdev.c | 4 +---
src/util/virnetdevbandwidth.c| 12 +++-
src/util/virnetdevmacvlan.c | 4 ++--
src/util/virnetdevvlan.c | 4 +---
src/util/virnetdevvportprofile.c | 4 +---
src/util/virnetlink.c| 8 ++--
src/util/virobject.c | 9 -
src/util/virpci.c| 13 +++--
src/util/virprocess.c| 4 +---
src/util/virsexpr.c | 4 +---
src/util/virstoragefile.c| 24 ++--
src/util/virstring.c | 5 +++--
src/util/virsysinfo.c| 8 +++-
src/util/virthreadpool.c | 21 +
src/util/virtime.c | 8 ++--
src/util/virtypedparam.c | 36 +---
src/util/viruri.c| 2 +-
src/util/virusb.c| 8 ++--
src/util/virutil.c | 36
src/util/virxml.c| 1 -
30 files changed, 81 insertions(+), 221 deletions(-)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index aa55a33..c8ec805 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -609,7 +609,6 @@ virXPathNodeSet(const char *xpath,
ret = obj-nodesetval-nodeNr;
if (list != NULL ret) {
if (VIR_ALLOC_N(*list, ret) 0) {
-virReportOOMError();
ret = -1;
} else {
memcpy(*list, obj-nodesetval-nodeTab,
Maybe an sed script could help here at least in removing the
virReportOOMError, not so easily in removing the '{' and '}'.
Now I doubt we will see lots of OOM errors, but there are a lot more
that may now report twice ...
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] Rename virCgroupGetAppRoot to virCgroupForSelf
On 03/22/2013 10:28 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The virCgroupGetAppRoot is not clear in its meaning. Change
to virCgroupForSelf to highlight that this returns the
cgroup config for the caller's process
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 2 +-
src/lxc/lxc_cgroup.c | 2 +-
src/util/vircgroup.c | 9 ++---
src/util/vircgroup.h | 2 +-
4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 21bc615..f75d681 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1090,9 +1090,9 @@ virCgroupDenyDevicePath;
virCgroupForDomain;
virCgroupForDriver;
virCgroupForEmulator;
+virCgroupForSelf;
virCgroupForVcpu;
virCgroupFree;
-virCgroupGetAppRoot;
virCgroupGetBlkioWeight;
virCgroupGetCpuacctPercpuUsage;
virCgroupGetCpuacctStat;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index df468da..33c305a 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -293,7 +293,7 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo)
int ret;
virCgroupPtr cgroup;
-ret = virCgroupGetAppRoot(cgroup);
+ret = virCgroupForSelf(cgroup);
if (ret 0) {
virReportSystemError(-ret, %s,
_(Unable to get cgroup for container));
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 6998f13..266cecb 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -967,19 +967,22 @@ int virCgroupForDriver(const char *name ATTRIBUTE_UNUSED,
#endif
/**
-* virCgroupGetAppRoot:
+* virCgroupForSelf:
*
* @group: Pointer to returned virCgroupPtr
*
+* Obtain a cgroup representing the config of the
+* current process
+*
* Returns 0 on success
*/
#if defined HAVE_MNTENT_H defined HAVE_GETMNTENT_R
-int virCgroupGetAppRoot(virCgroupPtr *group)
+int virCgroupForSelf(virCgroupPtr *group)
{
return virCgroupNew(/, group);
}
#else
-int virCgroupGetAppRoot(virCgroupPtr *group ATTRIBUTE_UNUSED)
+int virCgroupForSelf(virCgroupPtr *group ATTRIBUTE_UNUSED)
{
return -ENXIO;
}
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index ea42fa2..45a2006 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -49,7 +49,7 @@ int virCgroupForDriver(const char *name,
bool privileged,
bool create);
-int virCgroupGetAppRoot(virCgroupPtr *group);
+int virCgroupForSelf(virCgroupPtr *group);
int virCgroupForDomain(virCgroupPtr driver,
const char *name,
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 2/2] util: Don't report OOM twice
On 22.03.2013 18:32, Stefan Berger wrote:
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
Adapt code under src/util/ to fact, that VIR_ALLOC* now reports OOM
error. There is no need to report it twice now.
---
Maybe an sed script could help here at least in removing the
virReportOOMError, not so easily in removing the '{' and '}'.
Now I doubt we will see lots of OOM errors, but there are a lot more
that may now report twice ...
ACK
Yep. This is just a patch to show off how much we are going to save.
But now that I am thinking this over again - maybe virAsprintf deserves
to report OOM error as well (for virlog.c and virerror.c we are gonna
need virAsprintf variant without OOM error reporting). And probably a
wrapper over strdup as well - again with OOM error reporting. I just
don't know if I should do all the adaptation in one patch patch set or
in three different series.
BTW: I am not that strong in RE, so I am doing this with vim macros.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: probe for inverted ctdir
On Fri, Mar 22, 2013 at 08:26:59AM -0400, Stefan Berger wrote: Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT. If the sent byte arrives, the newer netfilter implementation has been detected. I think this is really very hackish. If this test capability goes wrong for any reason, then we're going to silently setting up incorrect rules, which would be a security flaw. I think we need a more robust detection system for this. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: Don't set address type too early during virtio disk hotplug
f946462e14ac036357b7c11ce5c23f94a3ee4e49 changed behavior by settings
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI upfront. If we do so before invoking
qemuDomainPCIAddressEnsureAddr we merely try to set the PCI slot via
qemuDomainPCIAddressReserveSlot instead reserving a new address via
qemuDomainPCIAddressSetNextAddr which fails with
$ ~/run-tck-test domain/200-disk-hotplug.t
./scripts/domain/200-disk-hotplug.t .. # Creating a new transient domain
./scripts/domain/200-disk-hotplug.t .. 1/5 # Attaching the new disk
/var/lib/jenkins/jobs/libvirt-tck-build/workspace/scratchdir/200-disk-hotplug/extra.img
# Failed test 'disk has been attached'
# at ./scripts/domain/200-disk-hotplug.t line 67.
# died: Sys::Virt::Error (libvirt error code: 1, message: internal error
unable to reserve PCI address 0:0:0.0
# )
---
v2 merely fixes a typo in $subject.
-- Guido
src/qemu/qemu_hotplug.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index de9edd4..b978b97 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -226,7 +226,6 @@ int qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
else if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_VIRTIO_S390))
disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
-else disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
}
for (i = 0 ; i vm-def-ndisks ; i++) {
@@ -253,7 +252,8 @@ int qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (qemuDomainCCWAddressAssign(disk-info, priv-ccwaddrs,
!disk-info.addr.ccw.assigned) 0)
goto error;
-} else if (disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+} else if (!disk-info.type ||
+disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
if (qemuDomainPCIAddressEnsureAddr(priv-pciaddrs, disk-info)
0)
goto error;
}
@@ -291,14 +291,17 @@ int qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
}
}
}
-} else if (disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI){
+} else if (!disk-info.type ||
+disk-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
virDevicePCIAddress guestAddr = disk-info.addr.pci;
ret = qemuMonitorAddPCIDisk(priv-mon,
disk-src,
type,
guestAddr);
-if (ret == 0)
+if (ret == 0) {
+disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
memcpy(disk-info.addr.pci, guestAddr, sizeof(guestAddr));
+}
}
qemuDomainObjExitMonitor(driver, vm);
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: probe for inverted ctdir
On 03/22/2013 01:49 PM, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 08:26:59AM -0400, Stefan Berger wrote: Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT. If the sent byte arrives, the newer netfilter implementation has been detected. I think this is really very hackish. If this test capability goes wrong for any reason, then we're going to silently setting up incorrect rules, which would be a security flaw. I think we need a more robust detection system for this. What method do you suggest? We cannot look in the kernel afaik so we have to probe it. We have to setup some rules to make this work and the tighter the rules are the better, so they don't interfer with anything else. Then of course we need to send 'something' so we can detect whether this is an old or a new system. We could of course send once with the rules with '--ctdir ORIGINAL' and then do a 2nd test with '--ctdir REPLY' to make sure that it works at all. But what would we do if we detected that it doesn't work in either one of the cases? Terminate? The assumption of the code was that we have a working loopback device. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: probe for inverted ctdir
On Fri, Mar 22, 2013 at 01:58:50PM -0400, Stefan Berger wrote: On 03/22/2013 01:49 PM, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 08:26:59AM -0400, Stefan Berger wrote: Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT. If the sent byte arrives, the newer netfilter implementation has been detected. I think this is really very hackish. If this test capability goes wrong for any reason, then we're going to silently setting up incorrect rules, which would be a security flaw. I think we need a more robust detection system for this. What method do you suggest? We cannot look in the kernel afaik so we have to probe it. We have to setup some rules to make this work and the tighter the rules are the better, so they don't interfer with anything else. Then of course we need to send 'something' so we can detect whether this is an old or a new system. We could of course send once with the rules with '--ctdir ORIGINAL' and then do a 2nd test with '--ctdir REPLY' to make sure that it works at all. But what would we do if we detected that it doesn't work in either one of the cases? Terminate? The assumption of the code was that we have a working loopback device. When was the change in semantics made to iptables ? Is this considered a critical bug by iptables maintainers, if so then we could just use the correct syntax unconditionally, and let distro vendors fix their iptables binaries to work correctly, instead of trying to workaround the brokeness. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: probe for inverted ctdir
On 03/22/2013 02:04 PM, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 01:58:50PM -0400, Stefan Berger wrote: On 03/22/2013 01:49 PM, Daniel P. Berrange wrote: On Fri, Mar 22, 2013 at 08:26:59AM -0400, Stefan Berger wrote: Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT. If the sent byte arrives, the newer netfilter implementation has been detected. I think this is really very hackish. If this test capability goes wrong for any reason, then we're going to silently setting up incorrect rules, which would be a security flaw. I think we need a more robust detection system for this. What method do you suggest? We cannot look in the kernel afaik so we have to probe it. We have to setup some rules to make this work and the tighter the rules are the better, so they don't interfer with anything else. Then of course we need to send 'something' so we can detect whether this is an old or a new system. We could of course send once with the rules with '--ctdir ORIGINAL' and then do a 2nd test with '--ctdir REPLY' to make sure that it works at all. But what would we do if we detected that it doesn't work in either one of the cases? Terminate? The assumption of the code was that we have a working loopback device. When was the change in semantics made to iptables ? Is this considered a critical bug by iptables maintainers, if so then we could just use the correct syntax unconditionally, and let distro vendors fix their iptables binaries to work correctly, instead of trying to workaround the brokeness. The problem is libvirt can be running on any kernel with any mix of iptables tools on top of that kernel. It was fixed in the kernel a while ago actually and the tools are unaffected by this just that the interpretation of the '--ctdir ORIGINAL' parameter in the kernel now is reverted and before the change it was '--ctdir REPLY' that setup that same rule in the kernel. Admins using this -m conntrack option are going to love it. Install a new kernel, reboot and your previously stored filtering rules aren't working anymore. Laine pointed me to the thread: https://www.redhat.com/archives/libvirt-users/2013-March/msg00109.html A simple '!' causing headaches. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: probe for inverted ctdir
On 03/22/2013 08:26 AM, Stefan Berger wrote: Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT. If the sent byte arrives, the newer netfilter implementation has been detected. While this is an admirable piece of work :-), I'm concerned that it may 1) be fragile, and 2) assume too much about the system being probed, and end up giving incorrect results in some circumstances. But since we have the check in place, we would be lulled into believing that we always correctly know which version of --ctdir we're working with, and end up with a non-working system and no clear indication why. It's very distressing that so little thought was apparently put into the far-reaching effects of making such an ABI change to netfilter; in my mind it really does render --ctdir more or less unusable except for very controlled cases where the same people are maintaining both netfilter/kernel and libvirt for a particular release of a particular distro. I unfortunately also don't have any alternative to offer, other than just don't use it (although this message Pablo from netfilter says that can be done with no reduction in security): https://www.redhat.com/archives/libvirt-users/2013-March/msg00128.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] nwfilter: probe for inverted ctdir
On 03/22/2013 02:29 PM, Laine Stump wrote: On 03/22/2013 08:26 AM, Stefan Berger wrote: Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instead and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT. If the sent byte arrives, the newer netfilter implementation has been detected. While this is an admirable piece of work :-), I'm concerned that it may 1) be fragile, and 2) assume too much about the system being probed, and end up giving incorrect results in some circumstances. But since we have the check in place, we would be lulled into believing that we always correctly know which version of --ctdir we're working with, and end up with a non-working system and no clear indication why. So is the consensus now that it cannot be probed for in all cases by libvirt? What alternative do you suggest? Removal of --ctdir usage even if it was there for a reason? Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5] qemu: Allow migration over IPv6
Allow migration over IPv6 by listening on [::] instead of 0.0.0.0
when QEMU supports it (QEMU_CAPS_IPV6_MIGRATION) and there is
at least one v6 address configured on the system.
Use virURIParse in qemuMigrationPrepareDirect to allow parsing
IPv6 addresses, which would cause an 'incorrect :port' error
message before.
Move setting of migrateFrom from qemuMigrationPrepare{Direct,Tunnel}
after domain XML parsing, since we need the QEMU binary path from it
to get its capabilities.
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013
---
diff to v4:
Always listen on IPv6 if it's available.
Don't add a migration flag.
v4:
https://www.redhat.com/archives/libvir-list/2013-March/msg01213.html
discussion:
https://www.redhat.com/archives/libvir-list/2013-March/msg00515.html
src/qemu/qemu_capabilities.c | 6 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_migration.c| 104 +--
tests/qemuhelptest.c | 9 ++--
4 files changed, 93 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3840b41..1e1da4d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -213,6 +213,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
virtio-ccw,
dtb,
megasas,
+
+ ipv6-migration, /* 135 */
);
struct _virQEMUCaps {
@@ -1181,6 +1183,9 @@ virQEMUCapsComputeCmdFlags(const char *help,
if (version = 11000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_HOST);
+if (version = 1001000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION);
+
if (version = 1002000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
return 0;
@@ -2317,6 +2322,7 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX);
virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT);
virQEMUCapsSet(qemuCaps, QEMU_CAPS_DTB);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION);
}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 7101f67..2ccc7c2 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -174,6 +174,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_VIRTIO_CCW = 132, /* -device virtio-*-ccw */
QEMU_CAPS_DTB= 133, /* -dtb file */
QEMU_CAPS_SCSI_MEGASAS = 134, /* -device megasas */
+QEMU_CAPS_IPV6_MIGRATION = 135, /* -incoming [::] */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 537b834..867c7f1 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -22,6 +22,8 @@
#include config.h
+#include netdb.h
+#include sys/socket.h
#include sys/time.h
#ifdef WITH_GNUTLS
# include gnutls/gnutls.h
@@ -1104,12 +1106,12 @@ error:
*/
static int
qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
-virDomainObjPtr vm)
+virDomainObjPtr vm,
+const char *listenAddr)
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm-privateData;
unsigned short port = 0;
-const char *listenAddr = 0.0.0.0;
char *diskAlias = NULL;
size_t i;
@@ -1980,8 +1982,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
int *cookieoutlen,
const char *dname,
const char *dom_xml,
-const char *migrateFrom,
virStreamPtr st,
+unsigned int port,
unsigned long flags)
{
virDomainDefPtr def = NULL;
@@ -1997,6 +1999,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
char *xmlout = NULL;
unsigned int cookieFlags;
virCapsPtr caps = NULL;
+const char *listenAddr = NULL;
+char *migrateFrom = NULL;
if (virTimeMillisNow(now) 0)
return -1;
@@ -2084,6 +2088,45 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
}
}
+if (tunnel) {
+/* QEMU will be started with -incoming stdio
+ * (which qemu_command might convert to exec:cat or fd:n)
+ */
+if (!(migrateFrom = strdup(stdio))) {
+virReportOOMError();
+goto cleanup;
+}
+} else {
+virQEMUCapsPtr qemuCaps = NULL;
+struct addrinfo *info = NULL;
+struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG,
+ .ai_socktype = SOCK_STREAM };
+
+if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver-qemuCapsCache,
+def-emulator)))
+goto cleanup;
+
+/* Listen on :: instead of 0.0.0.0 if QEMU understands it
+ * and there is at least one IPv6 address configured
+ */
+if
[libvirt] [PATCH v3] nwfilter: probe for inverted ctdir
Linux netfilter at some point inverted the meaning of the '--ctdir reply'
and newer netfilter implementations now expect '--ctdir original'
instead and vice-versa.
We probe for this netfilter change via a UDP message over loopback and 3
filtering rules applied to INPUT two times, one time with '--ctdir original'
which should then work on 'fixed' netfilter and one other time with
'--ctdir reply' which should only work on the 'old' netfilter.
If neither one of the tests gets the data through, then the loopback device
is probably not configured correctly. If both tests get the data through
something must be seriously wrong. In both of these two latter cases
no '--ctdir' will then be applied to the rules.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
v2-v3:
- probing with --ctdir original and --ctdir reply
v1-v2:
- using virSocketAddrParseIPv4
---
src/nwfilter/nwfilter_ebiptables_driver.c | 169
++
1 file changed, 169 insertions(+)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -27,6 +27,10 @@
#include string.h
#include sys/stat.h
#include fcntl.h
+#include arpa/inet.h
+#include sys/select.h
+#include sys/time.h
+#include unistd.h
#include internal.h
@@ -85,6 +89,17 @@ static char *iptables_cmd_path;
static char *ip6tables_cmd_path;
static char *grep_cmd_path;
+/*
+ * --ctdir original vs. --ctdir reply's meaning was inverted in netfilter
+ * at some point. We probe for it.
+ */
+enum ctdirStatus {
+CTDIR_STATUS_UNKNOWN= 0,
+CTDIR_STATUS_CORRECTED = (1 0),
+CTDIR_STATUS_OLD= (1 1),
+};
+static enum ctdirStatus iptables_ctdir_corrected;
+
#define PRINT_ROOT_CHAIN(buf, prefix, ifname) \
snprintf(buf, sizeof(buf), libvirt-%c-%s, prefix, ifname)
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
@@ -1262,6 +1277,17 @@ iptablesEnforceDirection(int directionIn
virNWFilterRuleDefPtr rule,
virBufferPtr buf)
{
+switch (iptables_ctdir_corrected) {
+case CTDIR_STATUS_UNKNOWN:
+/* could not be determined or s.th. is seriously wrong */
+return;
+case CTDIR_STATUS_CORRECTED:
+directionIn = !directionIn;
+break;
+case CTDIR_STATUS_OLD:
+break;
+}
+
if (rule-tt != VIR_NWFILTER_RULE_DIRECTION_INOUT)
virBufferAsprintf(buf, -m conntrack --ctdir %s,
(directionIn) ? Original
@@ -4304,6 +4330,146 @@ ebiptablesDriverTestCLITools(void)
return ret;
}
+static void
+ebiptablesDriverProbeCtdir(void)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+static const char cmdline[] =
+ $IPT -%c INPUT %c -i lo -p udp --dport %hu
+ -m state --state ESTABLISHED -j ACCEPT CMD_SEPARATOR
+ $IPT -%c INPUT %c -i lo -p udp --dport %hu
+ -m conntrack --ctdir %s -j ACCEPT CMD_SEPARATOR
+ $IPT -%c INPUT %c -i lo -p udp --dport %hu -j DROP;
+/*
+ * Above '--ctdir original' gets this test to receive a message on
+ * 'fixed' netfilter.
+ */
+unsigned short port;
+int ssockfd = -1, csockfd = -1;
+virSocketAddr saddr;
+struct sockaddr_in *serveraddr = saddr.data.inet4;
+fd_set readfds;
+struct timeval timeout = {
+.tv_sec = 0,
+.tv_usec = 1000 * 200,
+};
+int n, i, results = 0;
+const char *ctdiropts[2] = { original, reply };
+unsigned char data[10];
+
+if (virSocketAddrParseIPv4(saddr, 127.0.0.1) 0) {
+VIR_ERROR(_(Could not parse IP address));
+goto cleanup;
+}
+
+if ((ssockfd = socket(AF_INET, SOCK_DGRAM, 0)) 0 ||
+(csockfd = socket(AF_INET, SOCK_DGRAM, 0)) 0) {
+ VIR_ERROR(_(Could not open UDP socket));
+ goto cleanup;
+}
+
+for (port = 0x; port 1024; port--) {
+serveraddr-sin_port = htons(port);
+if (bind(ssockfd, (struct sockaddr *)serveraddr,
+ sizeof(*serveraddr)) == 0)
+break;
+}
+if (port == 1024) {
+VIR_ERROR(_(Could not bind to any UDP socket));
+goto cleanup;
+}
+
+i = 0;
+while (true) {
+NWFILTER_SET_IPTABLES_SHELLVAR(buf);
+virBufferAsprintf(buf, cmdline,
+ 'I', '1', port,
+ 'I', '2', port, ctdiropts[i],
+ 'I', '3', port);
+
+if (virBufferError(buf)) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (ebiptablesExecCLI(buf, NULL, NULL) 0) {
+VIR_ERROR(_(Could not apply iptables rules));
+goto cleanup_iptables;
+}
+
+virBufferFreeAndReset(buf);
+
+if (sendto(csockfd, cmdline, 1, 0, (struct sockaddr *)serveraddr,
+ sizeof(*serveraddr))
Re: [libvirt] [PATCH] Make virsh support '~' and '$HOME' in interactive mode
On 03/22/2013 03:34 AM, Christophe Fergeau wrote: On Thu, Mar 21, 2013 at 09:43:02PM -0600, Eric Blake wrote: Expanding everything means re-implementing what the shell does. wordexp() would be ideal for this, except that wordexp() is not portable enough. Maybe wordexp() could be used on platform which support it, and on platforms that do not have it, it would not be used, and env var would not be expanded? Or do we want to have identical behaviour for virsh on all platforms to avoid confusion? I think the confusion aspect must not be underestimated. If I write a virsh script on Linux, and then port it to a Windows machine, I would still want it to have the same effect on my guests. But if I relied on expansions, which happened to work on Linux but are missing on mingw, my script would change semantics. Also, we strive hard to avoid backward-incompatible changes; a script that was written before expansions, but which used unquoted '$', should still work after expansions are added. Maybe we could make expansions be off by default, and provide a new virsh command to opt-in; but at that point, we are back to the question of why not script the shell to do it in the first place. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Fix docs for virsh setmaxmem
On 03/22/2013 04:08 AM, Peter Krempa wrote: The docs assumed the command works always for QEMU and other hypervisors. Unfortunately until qemu will add memory hotplug this can't be done. Fix the docs to mention this limitation. The setmaxmem command controls balloon size, not memory hotplug. If qemu adds memory hotplug, we STILL have to pre-declare a maximum memory size when qemu first boots, and at runtime, you can only change the current memory. And if we do add qemu memory hotplug support (and not just memory ballooning), I'm not sure if it would make sense to reuse the setmaxmem command (probably with a new flag) or add a new command. --- tools/virsh.pod | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -This command works for at least the Xen, QEMU/KVM and vSphere/ESX hypervisors. +Some hypervisors such as QEMU/KVM don't support live changes (especially +increasing) of the maximum memory limit. I don't know of any hypervisor that supports changing the maximum limit on a live domain - the maximum is pinned when the hypervisor starts, and can only be changed for the next boot. At any rate, while this wording might not be the best possible, it is certainly an improvement, so: ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: fix unlikely memory leak in rbd backend
On 03/18/2013 06:28 PM, Eric Blake wrote: On 03/18/2013 02:07 PM, Laine Stump wrote: virStorageBackendRBDRefreshPool() first allocates an array big enough to hold 1024 names, then calls rbd_list(), which returns ERANGE if the array isn't big enough. When that happens, the VIR_ALLOC_N is called again with a larger size. Unfortunately, the original array isn't freed before allocating a new one. --- src/storage/storage_backend_rbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 8a0e517..e815192 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -317,6 +317,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_WARN(%s, _(A problem occurred while listing RBD images)); goto cleanup; } +VIR_FREE(names); This works, but is possibly less efficient than using VIR_REALLOC_N instead of VIR_ALLOC_N in the first place. I had thought of that, but figured that internally it would likely be the same operation as a free + new malloc, but would also do a copy from the old region to new, which is pointless in this case, since the old memory hasn't been set to anything and will be immediately overwritten anyway. ACK, since it's not on the hot path. I'm pushing as is. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/2] Report OOM on VIR_ALLOC failure
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
Currently, our code is plenty of following scheme:
if (VIR_ALLOC(dummyPtr) 0) {
virReportOOMError();
goto cleanup;
}
or something similar. What if we just move the OOM reporting into
VIR_ALLOC?
One thing you might want to consider is the case where the OOM is caused
by some bug in the code, for example incorrectly computing the size of a
buffer to allocate, or a loop that keeps allocating again and again
until failure. Currently we might get some useful information in the
form of the function and line number where the error occurred. With your
change that information would be lost.
Likely the times when this would be helpful would be extremely rare, but
just in case, you might want to think about having the VIR_ALLOC* macros
send __FILE__, __FUNCTION__, and __LINE__ to virAlloc* to be passed on
to a new variation of virReportOOMError() that takes those as args
rather than filling them out inline.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
