Re: [libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Eric Blake]

On 12/20/2013 11:17 AM, Eric Blake wrote:
> On 12/20/2013 09:34 AM, Daniel P. Berrange wrote:
>> On Fri, Dec 20, 2013 at 08:24:41PM +0400, Reco wrote:
>>> Implement virProcessRunInMountNamespace, which runs callback of type
>>> virProcessNamespaceCallback in a container namespace.
>>>
>>> Hope it'll nail it this time.
> 
> Fails 'make syntax-check':
> 
> prohibit_fork_wrappers
> src/util/virprocess.c:874:switch (cpid = fork()) {
> maint.mk: use virCommand for child processes
> make: *** [sc_prohibit_fork_wrappers] Error 1
> 
>>> +
>>> +switch (cpid = fork()) {

Yuck.  Rewriting this to use virFork() has exposed some nastiness in our
existing library: 'virsh lxc-enter-namespace' and 'virt-login-shell'
both use virFork() incorrectly, which is a sign that the interface
itself is to blame.  The fact that a child process must explicitly call
_exit() if virFork() returns < 0 but set *pid to 0 is just nasty - it
violates the normal expectation that a negative return has no more work
to do.  I still plan on posting a revised version of this patch, but it
will be part of a bit bigger series that first tries to make virFork and
virProcessWait friendlier.

I spent some time learning about namespaces in the process - I can see
why 'virsh lxc-enter-namespace' must fork twice (although setns() can
join some namespaces on a per-thread basis, it cannot do so for the
'user namespace', so one fork is required to get to a single-threaded
state since virsh is multithreaded; the second fork is required because
setting the 'pid namespace' doesn't actually change the pid of the
current process, it only designates that child processes will be in the
new namespace).  But why is 'virt-login-shell' double-forking?  It is
already single-threaded, and doesn't do anything after fork except wait
on the child process, so a single fork after setting the 'pid namespace'
ought to be sufficient.

>>> +case 0:
>>> +if (setns(fd, 0) == -1) {
>>> +_exit(-1);
> 
> exit(-1) is wrong; the user will see $? of 255 (not -1), and it is not
> our typical exit status for failure (where we normally want to be < 128
> so that it is not confused with exit due to signal).  Besides, I hate
> magic numbers; better would be _exit(EXIT_FAILURE).

Or maybe _exit(127), which is more idiomatic for failures when fork()
succeeds but exec() is not reached (see posix_spawn() for example).

> 
>>> +default:
>>> +if (virProcessWait(cpid, &status) < 0 || status < 0) {
> 
> No need to pass &status to virProcessWait() if you are going to insist
> on 0 status anyways; better to pass NULL and let virProcessWait do the
> validation on your behalf.

On the other hand, we may NEED to pass status on to the user, as I
noticed in patch 2 that your callback function wants to distinguish
between fatal errors and the simpler case of no initctl support;
collapsing all non-zero status into failure loses our chance to report
multiple bits of information.

> I'll repost a full v5 series with my proposed fixes to these issues in a
> separate thread, and wait for a review (since this fixes a CVE, it needs
> a second set of eyes).

That, and we still need the fix for virDomainDeviceAttach using /dev
only within the guest namespace.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/1] virsh nodecpustats erroneously returns stats for offline cpus on Linux

[Bing Bu Cao]

On 12/20/2013 05:47 PM, Pradipta Kumar Banerjee wrote:

[snip]

   for (i = 0; i < *nparams; i++) {
   virNodeCPUStatsPtr param = ¶ms[i];


What about this?

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..aa1ad81 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -637,8 +637,9 @@ linuxNodeGetCPUStats(FILE *procstat,

   while (fgets(line, sizeof(line), procstat) != NULL) {
   char *buf = line;
+char **buf_header = virStringSplit(buf, " ", 2);

-if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
+if (STREQ(buf_header[0], cpu_header)) { /* aka logical CPU time */
   size_t i;

   if (sscanf(buf,
@@ -697,6 +698,7 @@ linuxNodeGetCPUStats(FILE *procstat,
   ret = 0;
   goto cleanup;
   }
+virStringFreeList(buf_header);
   }

   virReportInvalidArg(cpuNum,



This is definitely better and lesser amount of code..


I think the version with virStringSplit would need some fine tuning since in its
current form it will not free the memory for the failure cases..Comments ?

Good point.
We should add virStringFreeList(buf_header) to cleanup.


Also can some expert here provide some tips on whether in this particular
circumstance it should be fine to allocate/realloc/free memory inside the while
loop.  Would be very helpful..

I think putting allocate/free memory inside the while is OK for me.
Because we must loop for searching the expected line,the times of loop 
depends on the content of /proc/stat.

However, as you said, we better get some advise from the experts here.



Thanks,
Pradipta



Thanks




--
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list










--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list




--
Best Regards,
Bing Bu Cao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/5] qemu: Do not access stale data in virDomainBlockStats

[Eric Blake]

On 12/20/2013 02:36 PM, Jiri Denemark wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1043069
> 
> When virDomainDetachDeviceFlags is called concurrently to
> virDomainBlockStats: libvirtd may crash because qemuDomainBlockStats
> finds a disk in vm->def before getting a job on a domain and uses the
> disk pointer after getting the job. However, the domain in unlocked
> while waiting on a job condition and thus data behind the disk pointer
> may disappear. This happens when thread 1 runs
> virDomainDetachDeviceFlags and enters monitor to actually remove the
> disk. Then another thread starts running virDomainBlockStats, finds the
> disk in vm->def, and while it's waiting on the job condition (owned by
> the first thread), the first thread finishes the disk removal. When the
> second thread gets the job, the memory pointed to be the disk pointer is
> already gone.
> 
> That said, every API that is going to begin a job should do that before
> fetching data from vm->def.

Bummer, we'll need a CVE for this.  It's been present since at least
0.9.x days.

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 3/3] libvirt: Implement two-tier driver loading

[Adam Walters]

This implements a two-tier driver loading system into libvirt. The two classes 
of drivers are "Libvirt" drivers and "Hypervisor" drivers. Hypervisor drivers 
are fairly self-explanatory, they provide domain services. Libvirt drivers are 
sort of the backend drivers for those, like the secret and storage drivers. In 
the two-tier loading system here, the "Libvirt" drivers are all loaded and 
auto-started. Once those have finished, the "Hypervisor" drivers are loaded and 
auto-started. By doing things in this manner, we do not have to hard-code a 
driver loading order or roll our own dynamic dependency-based loading 
algorithm, while still gaining the benefits of a more orderly driver loading 
approach, which should help minimize the possibility of a race condition during 
startup. If another race condition is found, the code can be extended to 
provide any number of extra tiers without much trouble.

Signed-off-by: Adam Walters 
---
 src/libvirt.c | 57 +
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 77f481e..9c00491 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -837,31 +837,72 @@ int virStateInitialize(bool privileged,
void *opaque)
 {
 size_t i;
+virStateDriverPtr virLibvirtStateDriverTab[MAX_DRIVERS];
+int virLibvirtStateDriverTabCount = 0;
+virStateDriverPtr virHypervisorStateDriverTab[MAX_DRIVERS];
+int virHypervisorStateDriverTabCount = 0;
 
 if (virInitialize() < 0)
 return -1;
 
 for (i = 0; i < virStateDriverTabCount; i++) {
-if (virStateDriverTab[i]->stateInitialize) {
+switch (virStateDriverTab[i]->stateType) {
+case VIR_DRV_STATE_DRV_LIBVIRT:
+virLibvirtStateDriverTab[virLibvirtStateDriverTabCount++] =
+virStateDriverTab[i];
+break;
+case VIR_DRV_STATE_DRV_HYPERVISOR:
+virHypervisorStateDriverTab[virHypervisorStateDriverTabCount++] =
+virStateDriverTab[i];
+break;
+}
+}
+
+for (i = 0; i < virLibvirtStateDriverTabCount; i++) {
+if (virLibvirtStateDriverTab[i]->stateInitialize) {
 VIR_DEBUG("Running global init for %s state driver",
-  virStateDriverTab[i]->name);
-if (virStateDriverTab[i]->stateInitialize(privileged,
+  virLibvirtStateDriverTab[i]->name);
+if (virLibvirtStateDriverTab[i]->stateInitialize(privileged,
   callback,
   opaque) < 0) {
 virErrorPtr err = virGetLastError();
 VIR_ERROR(_("Initialization of %s state driver failed: %s"),
-  virStateDriverTab[i]->name,
+  virLibvirtStateDriverTab[i]->name,
   err && err->message ? err->message : _("Unknown 
problem"));
 return -1;
 }
 }
 }
 
-for (i = 0; i < virStateDriverTabCount; i++) {
-if (virStateDriverTab[i]->stateAutoStart) {
+for (i = 0; i < virLibvirtStateDriverTabCount; i++) {
+if (virLibvirtStateDriverTab[i]->stateAutoStart) {
+VIR_DEBUG("Running global auto start for %s state driver",
+  virLibvirtStateDriverTab[i]->name);
+virLibvirtStateDriverTab[i]->stateAutoStart();
+}
+}
+
+for (i = 0; i < virHypervisorStateDriverTabCount; i++) {
+if (virHypervisorStateDriverTab[i]->stateInitialize) {
+VIR_DEBUG("Running global init for %s state driver",
+  virHypervisorStateDriverTab[i]->name);
+if (virHypervisorStateDriverTab[i]->stateInitialize(privileged,
+  callback,
+  opaque) < 0) {
+virErrorPtr err = virGetLastError();
+VIR_ERROR(_("Initialization of %s state driver failed: %s"),
+  virHypervisorStateDriverTab[i]->name,
+  err && err->message ? err->message : _("Unknown 
problem"));
+return -1;
+}
+}
+}
+
+for (i = 0; i < virHypervisorStateDriverTabCount; i++) {
+if (virHypervisorStateDriverTab[i]->stateAutoStart) {
 VIR_DEBUG("Running global auto start for %s state driver",
-  virStateDriverTab[i]->name);
-virStateDriverTab[i]->stateAutoStart();
+  virHypervisorStateDriverTab[i]->name);
+virHypervisorStateDriverTab[i]->stateAutoStart();
 }
 }
 return 0;
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 0/3] Implement two-tier driver loading

[Adam Walters]

This patchset implements a two-tier driver loading system. I split the 
hypervisor drivers out into their own tier, which is loaded after the other 
drivers. This has the net effect of ensuring that things like secrets, 
networks, etc., are initialized and auto-started before any hypervisors, such 
as qemu, lxc, etc., are touched. This resolves the race condition present when 
starting libvirtd while domains are running, which happens when restarting 
libvirtd after having started at least one domain.

This patch will work without my config driver patchset, but does prevent RBD 
storage pools from auto-starting. It may also affect other pool types, but I 
only have file and RBD to test with, personally. The RBD storage pool is only 
affected because it requires a hypervisor connection (prior to this patchset, 
that connection was hardcoded to be a connection to qemu on localhost) in order 
to look up secrets. Any pool type that does not use/need data outside of the 
base storage pool definition should continue to auto-start (file backed pools 
definitely still work) and also no longer be part of the restart race condition.

For anyone who is not familiar with the race condition I mentioned above, the 
basic description is the upon restarting libvirtd, any running QEMU domains 
using storage pool backed disks are killed (randomly) due to their storage pool 
not being online. This is due to storage pool auto-start not having finished 
before QEMU initalization runs.


I would appreciate any comments and suggestions about this patchset. It works 
for me on 4 machines running three different distros of Linux (Archlinux, 
Gentoo, and CentOS), so I would imagine it should work most anywhere.

Adam Walters (3):
  driver: Implement new state driver field
  storage: Fix hardcoded qemu connection
  libvirt: Implement two-tier driver loading

 src/config/config_driver.c  |  1 +
 src/driver.h|  6 
 src/interface/interface_backend_netcf.c |  1 +
 src/libvirt.c   | 57 -
 src/libxl/libxl_driver.c|  1 +
 src/lxc/lxc_driver.c|  1 +
 src/network/bridge_driver.c |  1 +
 src/node_device/node_device_hal.c   |  1 +
 src/node_device/node_device_udev.c  |  1 +
 src/nwfilter/nwfilter_driver.c  |  1 +
 src/qemu/qemu_driver.c  |  1 +
 src/remote/remote_driver.c  |  1 +
 src/secret/secret_driver.c  |  1 +
 src/storage/storage_driver.c| 13 
 src/uml/uml_driver.c|  1 +
 src/xen/xen_driver.c|  1 +
 16 files changed, 75 insertions(+), 14 deletions(-)

-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 2/3] storage: Fix hardcoded qemu connection

[Adam Walters]

This utilizes the config driver I submitted to resolve the hardcoded qemu 
connection string. With this, the storage driver no longer has a circular 
dependency with QEMU. Without this patch, when libvirtd is restarted, QEMU 
requires storage (when domains are using storage pool backings) and storage 
requires QEMU. This causes issues during startup.

Signed-off-by: Adam Walters 
---
 src/storage/storage_driver.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index a614279..1077a66 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -70,12 +70,12 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {
 size_t i;
 virConnectPtr conn = NULL;
 
-/* XXX Remove hardcoding of QEMU URI */
-if (driverState->privileged)
-conn = virConnectOpen("qemu:///system");
-else
-conn = virConnectOpen("qemu:///session");
-/* Ignoring NULL conn - let backends decide */
+conn = virConnectOpen("config:///");
+/* Ignoring NULL conn - let backends decide.  *
+ * As long as the config driver is built, and *
+ * it should be, since it is default on and   *
+ * not configurable, a NULL conn should never *
+ * happen.
 
 for (i = 0; i < driver->pools.count; i++) {
 virStoragePoolObjPtr pool = driver->pools.objs[i];
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 7/7] Makefile: Add config driver to src/Makefile.am

[Adam Walters]

This completes the addition of the config driver to libvirt. The final piece 
here is to add the config driver into the Makefile (via automake) so that the 
driver is actually compiled and linked.

Signed-off-by: Adam Walters 
---
 src/Makefile.am | 25 +
 1 file changed, 25 insertions(+)

diff --git a/src/Makefile.am b/src/Makefile.am
index 57e163f..e0b3677 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -504,6 +504,7 @@ DRIVER_SOURCE_FILES = \
$(QEMU_DRIVER_SOURCES) \
$(REMOTE_DRIVER_SOURCES) \
$(SECRET_DRIVER_SOURCES) \
+   $(CONFIG_DRIVER_SOURCES) \
$(STORAGE_DRIVER_SOURCES) \
$(TEST_DRIVER_SOURCES) \
$(UML_DRIVER_SOURCES) \
@@ -523,6 +524,7 @@ STATEFUL_DRIVER_SOURCE_FILES = \
$(NWFILTER_DRIVER_SOURCES) \
$(QEMU_DRIVER_SOURCES) \
$(SECRET_DRIVER_SOURCES) \
+   $(CONFIG_DRIVER_SOURCES) \
$(STORAGE_DRIVER_SOURCES) \
$(UML_DRIVER_SOURCES) \
$(XEN_DRIVER_SOURCES) \
@@ -799,6 +801,9 @@ endif WITH_INTERFACE
 SECRET_DRIVER_SOURCES =\
secret/secret_driver.h secret/secret_driver.c
 
+CONFIG_DRIVER_SOURCES =\
+   config/config_driver.c config/config_driver.h
+
 # Storage backend specific impls
 STORAGE_DRIVER_SOURCES =   \
storage/storage_driver.h storage/storage_driver.c   \
@@ -1386,6 +1391,25 @@ endif WITH_DRIVER_MODULES
 libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES)
 endif WITH_SECRETS
 
+if WITH_CONFIG
+if WITH_DRIVER_MODULES
+mod_LTLIBRARIES += libvirt_driver_config.la
+else ! WITH_DRIVER_MODULES
+noinst_LTLIBRARIES += libvirt_driver_config.la
+# Stateful, so linked to daemon instead
+#libvirt_la_BUILT_LIBADD += libvirt_driver_config.la
+endif ! WITH_DRIVER_MODULES
+libvirt_driver_config_la_CFLAGS = \
+   -I$(top_srcdir)/src/access \
+   -I$(top_srcdir)/src/conf \
+   $(AM_CFLAGS)
+if WITH_DRIVER_MODULES
+libvirt_driver_config_la_LIBADD = ../gnulib/lib/libgnu.la
+libvirt_driver_config_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
+endif WITH_DRIVER_MODULES
+libvirt_driver_config_la_SOURCES = $(CONFIG_DRIVER_SOURCES)
+endif WITH_CONFIG
+
 # Needed to keep automake quiet about conditionals
 libvirt_driver_storage_impl_la_SOURCES =
 libvirt_driver_storage_impl_la_CFLAGS = \
@@ -1661,6 +1685,7 @@ EXTRA_DIST += 
\
$(SECURITY_DRIVER_SELINUX_SOURCES)  \
$(SECURITY_DRIVER_APPARMOR_SOURCES) \
$(SECRET_DRIVER_SOURCES)\
+   $(CONFIG_DRIVER_SOURCES)\
$(VBOX_DRIVER_EXTRA_DIST)   \
$(VMWARE_DRIVER_SOURCES)\
$(XENXS_SOURCES)\
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 6/7] configure: Add config driver to configure script

[Adam Walters]

This conditionally enables compilation of the config driver based on if we are 
building libvirtd or not. Since this is only needed for hypervisor modules 
during libvirtd startup, we don't need to bother compiling the config driver 
when only building the client.

Signed-off-by: Adam Walters 
---
 configure.ac | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/configure.ac b/configure.ac
index ddbcc8e..4834c51 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1606,6 +1606,16 @@ if test "$with_secrets" = "yes" ; then
 fi
 AM_CONDITIONAL([WITH_SECRETS], [test "$with_secrets" = "yes"])
 
+if test "$with_libvirtd" = "no" ; then
+  with_config=no
+else
+  with_config=yes
+fi
+if test "$with_config" = "yes" ; then
+AC_DEFINE_UNQUOTED([WITH_CONFIG], 1, [whether local config driver is 
enabled])
+fi
+AM_CONDITIONAL([WITH_CONFIG], [test "$with_config" = "yes"])
+
 
 AC_ARG_WITH([storage-dir],
   [AS_HELP_STRING([--with-storage-dir],
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 1/3] driver: Implement new state driver field

[Adam Walters]

This implements a new field in the virStateDriver struct. In order to prevent 
possible compilation issues, this patch also implements te new field in all of 
the existing drivers. Other than in driver.h, the changes are all a single line 
addition to the files.

Signed-off-by: Adam Walters 
---
 src/config/config_driver.c  | 1 +
 src/driver.h| 6 ++
 src/interface/interface_backend_netcf.c | 1 +
 src/libxl/libxl_driver.c| 1 +
 src/lxc/lxc_driver.c| 1 +
 src/network/bridge_driver.c | 1 +
 src/node_device/node_device_hal.c   | 1 +
 src/node_device/node_device_udev.c  | 1 +
 src/nwfilter/nwfilter_driver.c  | 1 +
 src/qemu/qemu_driver.c  | 1 +
 src/remote/remote_driver.c  | 1 +
 src/secret/secret_driver.c  | 1 +
 src/storage/storage_driver.c| 1 +
 src/uml/uml_driver.c| 1 +
 src/xen/xen_driver.c| 1 +
 15 files changed, 20 insertions(+)

diff --git a/src/config/config_driver.c b/src/config/config_driver.c
index a057300..a817c7a 100644
--- a/src/config/config_driver.c
+++ b/src/config/config_driver.c
@@ -228,6 +228,7 @@ static virStateDriver configStateDriver = {
 .stateInitialize = configStateInitialize,
 .stateCleanup = configStateCleanup,
 .stateReload = configStateReload,
+.stateType = VIR_DRV_STATE_DRV_LIBVIRT,
 };
 
 int configRegister(void) {
diff --git a/src/driver.h b/src/driver.h
index b6927ea..6863910 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1833,6 +1833,11 @@ typedef int
 typedef int
 (*virDrvStateStop)(void);
 
+typedef enum {
+VIR_DRV_STATE_DRV_LIBVIRT = 1,
+VIR_DRV_STATE_DRV_HYPERVISOR = 2,
+} virDrvStateDrvType;
+
 typedef struct _virStateDriver virStateDriver;
 typedef virStateDriver *virStateDriverPtr;
 
@@ -1843,6 +1848,7 @@ struct _virStateDriver {
 virDrvStateCleanup stateCleanup;
 virDrvStateReload stateReload;
 virDrvStateStop stateStop;
+virDrvStateDrvType stateType;
 };
 # endif
 
diff --git a/src/interface/interface_backend_netcf.c 
b/src/interface/interface_backend_netcf.c
index c4e18c4..8b910f9 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -1157,6 +1157,7 @@ static virStateDriver interfaceStateDriver = {
 .stateInitialize = netcfStateInitialize,
 .stateCleanup = netcfStateCleanup,
 .stateReload = netcfStateReload,
+.stateType = VIR_DRV_STATE_DRV_LIBVIRT,
 };
 
 int netcfIfaceRegister(void) {
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d91744f..cce7798 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4292,6 +4292,7 @@ static virStateDriver libxlStateDriver = {
 .stateAutoStart = libxlStateAutoStart,
 .stateCleanup = libxlStateCleanup,
 .stateReload = libxlStateReload,
+.stateType = VIR_DRV_STATE_DRV_HYPERVISOR,
 };
 
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e5298d1..3fe88c6 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4815,6 +4815,7 @@ static virStateDriver lxcStateDriver = {
 .stateAutoStart = lxcStateAutoStart,
 .stateCleanup = lxcStateCleanup,
 .stateReload = lxcStateReload,
+.stateType = VIR_DRV_STATE_DRV_HYPERVISOR,
 };
 
 int lxcRegister(void)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3e10758..be49c78 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3131,6 +3131,7 @@ static virStateDriver networkStateDriver = {
 .stateAutoStart  = networkStateAutoStart,
 .stateCleanup = networkStateCleanup,
 .stateReload = networkStateReload,
+.stateType = VIR_DRV_STATE_DRV_LIBVIRT,
 };
 
 int networkRegister(void) {
diff --git a/src/node_device/node_device_hal.c 
b/src/node_device/node_device_hal.c
index fafd520..23cee5e 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -827,6 +827,7 @@ static virStateDriver halStateDriver = {
 .stateInitialize = nodeStateInitialize, /* 0.5.0 */
 .stateCleanup = nodeStateCleanup, /* 0.5.0 */
 .stateReload = nodeStateReload, /* 0.5.0 */
+.stateType = VIR_DRV_STATE_DRV_LIBVIRT,
 };
 
 int
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 5d49968..6282072 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1816,6 +1816,7 @@ static virStateDriver udevStateDriver = {
 .stateInitialize = nodeStateInitialize, /* 0.7.3 */
 .stateCleanup = nodeStateCleanup, /* 0.7.3 */
 .stateReload = nodeStateReload, /* 0.7.3 */
+.stateType = VIR_DRV_STATE_DRV_LIBVIRT,
 };
 
 int udevNodeRegister(void)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index d21dd82..a427a0b 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -702,6 +702,7 @@ static virStateDri

[libvirt] [RFC PATCH 5/7] po: Add config_driver.c to POTFILES.in

[Adam Walters]

Adding config_driver.c to POTFILES.in to fix a syntax-check error.

Signed-off-by: Adam Walters 
---
 po/POTFILES.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 49dfc9c..0e23610 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -27,6 +27,7 @@ src/conf/snapshot_conf.c
 src/conf/storage_conf.c
 src/conf/storage_encryption_conf.c
 src/conf/virchrdev.c
+src/config/config_driver.c
 src/cpu/cpu.c
 src/cpu/cpu_generic.c
 src/cpu/cpu_map.c
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 0/7] Adding 'config' driver

[Adam Walters]

This patchset adds a driver named 'config' that allows access to configuration 
data, such as secret and storage definitions. This is a pre-requisite for my 
next patchset which resolves the race condition on libvirtd startup and the 
circular dependencies between QEMU and the storage driver.

The basic rationale behind this idea is that there exist circumstances under 
which a driver may need to access things such as secrets during a time at which 
there is no active connection to a hypervisor. Without a connection, the data 
can't be accessed currently. I felt that this was a much simpler solution to 
the problem that building new APIs that do not require a connection to operate.

This driver is technically what one may call a hypervisor driver, but it does 
not implement any domain operations. It simply exists to handle requests by 
drivers for access to informatino that would otherwise require a connection. 
The URI used for this driver is 'config:///' and has been tested working on 4 
different machines of mine, running three different distributions of Linux 
(Archlinux, Gentoo, and CentOS). Being a very simple driver, I would expect it 
to work pretty much anywhere.

I would love to hear any comments and suggestions you may have about this 
driver. At the very least this plus my next patchset resolves the startup race 
condition on my machine. If a more robust setup (likely a new internal API) is 
in the works, this driver could act as a band-aid to allow access to this type 
of data in the interim if a better resolution is a ways off.

Adam Walters (7):
  config: Adding source for the config driver
  config: Adding header for the config driver
  virterror: Adding a new VIR_FROM_ define
  libvirtd: Add config driver hooks
  po: Add config_driver.c to POTFILES.in
  configure: Add config driver to configure script
  Makefile: Add config driver to src/Makefile.am

 configure.ac|  10 ++
 daemon/libvirtd.c   |  21 ++--
 include/libvirt/virterror.h |   2 +
 po/POTFILES.in  |   1 +
 src/Makefile.am |  25 +
 src/config/config_driver.c  | 237 
 src/config/config_driver.h  |  44 
 src/util/virerror.c |   2 +
 8 files changed, 336 insertions(+), 6 deletions(-)
 create mode 100644 src/config/config_driver.c
 create mode 100644 src/config/config_driver.h

-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 1/7] config: Adding source for the config driver

[Adam Walters]

This is the source code to the config driver. This driver is a hypervisor 
driver that does not support any domain operations. The sole purpose of this 
driver is to allow access to various bits of configuration information, such as 
secret or network definitions, from the initialization and auto-start routines 
of other drivers. This is the first step toward breaking the circular 
dependencies present in QEMU and the Storage driver, in addition to preventing 
future cases.

Signed-off-by: Adam Walters 
---
 src/config/config_driver.c | 237 +
 1 file changed, 237 insertions(+)
 create mode 100644 src/config/config_driver.c

diff --git a/src/config/config_driver.c b/src/config/config_driver.c
new file mode 100644
index 000..a057300
--- /dev/null
+++ b/src/config/config_driver.c
@@ -0,0 +1,237 @@
+/*
+ * config_driver.c: core driver methods for accessing configs
+ *
+ * Copyright (C) 2013 Adam Walters
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Adam Walters 
+ */
+#include 
+#include 
+
+#include "internal.h"
+#include "datatypes.h"
+#include "driver.h"
+#include "virlog.h"
+#include "viralloc.h"
+#include "virerror.h"
+#include "virstring.h"
+#include "viraccessapicheck.h"
+#include "nodeinfo.h"
+#include "config_driver.h"
+
+#define VIR_FROM_THIS VIR_FROM_CONFIG
+
+virConfigDriverPtr config_driver = NULL;
+
+static int configStateCleanup(void) {
+if (config_driver == NULL)
+return -1;
+
+virObjectUnref(config_driver->closeCallbacks);
+
+virSysinfoDefFree(config_driver->hostsysinfo);
+
+virMutexDestroy(&config_driver->lock);
+VIR_FREE(config_driver);
+
+return 0;
+}
+
+static int configStateInitialize(bool privileged,
+ virStateInhibitCallback callback 
ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+if (!privileged) {
+VIR_INFO("Not running privileged, disabling driver");
+return 0;
+}
+
+if (VIR_ALLOC(config_driver) < 0)
+return -1;
+
+if (virMutexInit(&config_driver->lock) < 0) {
+VIR_ERROR(_("cannot initialize mutex"));
+VIR_FREE(config_driver);
+return -1;
+}
+
+config_driver->hostsysinfo = virSysinfoRead();
+
+if (!(config_driver->closeCallbacks = virCloseCallbacksNew()))
+goto cleanup;
+
+return 0;
+
+cleanup:
+configStateCleanup();
+return -1;
+}
+
+static int configStateReload(void) {
+return 0;
+}
+
+static virDrvOpenStatus configConnectOpen(virConnectPtr conn,
+  virConnectAuthPtr auth 
ATTRIBUTE_UNUSED,
+  unsigned int flags)
+{
+virDrvOpenStatus ret = VIR_DRV_OPEN_ERROR;
+virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+
+if (conn->uri != NULL) {
+/* If URI isn't 'config', decline the connection */
+if (conn->uri->scheme == NULL ||
+STRNEQ(conn->uri->scheme, "config")) {
+ret = VIR_DRV_OPEN_DECLINED;
+goto cleanup;
+}
+
+/* Allow remote driver to deal with URIs with hostname set */
+if (conn->uri->server != NULL) {
+ret = VIR_DRV_OPEN_DECLINED;
+goto cleanup;
+}
+
+if (config_driver == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("config state driver is not active"));
+goto cleanup;
+}
+} else {
+ret = VIR_DRV_OPEN_DECLINED;
+goto cleanup;
+}
+
+if (virConnectOpenEnsureACL(conn) < 0)
+goto cleanup;
+
+conn->privateData = config_driver;
+
+ret = VIR_DRV_OPEN_SUCCESS;
+cleanup:
+return ret;
+}
+
+static int configConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED)
+{
+/* Cleanup callbacks on config_driver */
+/* Set conn->privateData to NULL */
+return 0;
+}
+
+static int
+configConnectSupportsFeature(virConnectPtr conn, int feature ATTRIBUTE_UNUSED)
+{
+if (virConnectSupportsFeatureEnsureACL(conn) < 0)
+return -1;
+
+return 0;
+}
+
+static const char *configConnectGetType(virConnectPtr conn ATTRIBUTE_UNUSED) {
+if (virConnectGetTypeEnsureACL(conn) < 0)
+return NULL;
+
+return "config";
+}
+
+static int configConnectIsSecure(virConnectPt

[libvirt] [RFC PATCH 2/7] config: Adding header for the config driver

[Adam Walters]

This is the header file for the config driver.

Signed-off-by: Adam Walters 
---
 src/config/config_driver.h | 44 
 1 file changed, 44 insertions(+)
 create mode 100644 src/config/config_driver.h

diff --git a/src/config/config_driver.h b/src/config/config_driver.h
new file mode 100644
index 000..c74af71
--- /dev/null
+++ b/src/config/config_driver.h
@@ -0,0 +1,44 @@
+/*
+ * config_driver.h: core driver methods for accessing configs
+ *
+ * Copyright (C) 2013 Adam Walters
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Adam Walters 
+ */
+
+#ifndef __CONFIG_DRIVER_H__
+# define __CONFIG_DRIVER_H__
+
+# include "virsysinfo.h"
+# include "virclosecallbacks.h"
+
+typedef struct _virConfigDriver virConfigDriver;
+typedef virConfigDriver *virConfigDriverPtr;
+
+struct _virConfigDriver {
+virMutex lock;
+
+/* Immutable pointer, lockless APIs*/
+virSysinfoDefPtr hostsysinfo;
+
+/* Immutable pointer, self-locking APIs*/
+virCloseCallbacksPtr closeCallbacks;
+};
+
+int configRegister(void);
+
+#endif /* __CONFIG_DRIVER_H__ */
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 4/7] libvirtd: Add config driver hooks

[Adam Walters]

This patch adds the config driver hooks and moves the secret driver hook 
definitions higher on the list. The secret driver move isn't strictly needed, 
but the comments state that these should be in preferred load order. Since 
other drivers might utilize the secret driver, it makes sense to have it loaded 
earlier in the startup sequence.

Signed-off-by: Adam Walters 
---
 daemon/libvirtd.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 49c42ad..251c2f4 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -95,6 +95,9 @@
 # ifdef WITH_NWFILTER
 #  include "nwfilter/nwfilter_driver.h"
 # endif
+# ifdef WITH_CONFIG
+#  include "config/config_driver.h"
+# endif
 #endif
 
 #include "configmake.h"
@@ -369,18 +372,21 @@ static void daemonInitialize(void)
  * If they try to open a connection for a module that
  * is not loaded they'll get a suitable error at that point
  */
+# ifdef WITH_CONFIG
+virDriverLoadModule("config");
+# endif
 # ifdef WITH_NETWORK
 virDriverLoadModule("network");
 # endif
+# ifdef WITH_SECRETS
+virDriverLoadModule("secret");
+# endif
 # ifdef WITH_STORAGE
 virDriverLoadModule("storage");
 # endif
 # ifdef WITH_NODE_DEVICES
 virDriverLoadModule("nodedev");
 # endif
-# ifdef WITH_SECRETS
-virDriverLoadModule("secret");
-# endif
 # ifdef WITH_NWFILTER
 virDriverLoadModule("nwfilter");
 # endif
@@ -406,21 +412,24 @@ static void daemonInitialize(void)
 virDriverLoadModule("vbox");
 # endif
 #else
+# ifdef WITH_CONFIG
+configRegister();
+# endif
 # ifdef WITH_NETWORK
 networkRegister();
 # endif
 # ifdef WITH_INTERFACE
 interfaceRegister();
 # endif
+# ifdef WITH_SECRETS
+secretRegister();
+# endif
 # ifdef WITH_STORAGE
 storageRegister();
 # endif
 # ifdef WITH_NODE_DEVICES
 nodedevRegister();
 # endif
-# ifdef WITH_SECRETS
-secretRegister();
-# endif
 # ifdef WITH_NWFILTER
 nwfilterRegister();
 # endif
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 3/7] virterror: Adding a new VIR_FROM_ define

[Adam Walters]

This patch adds VIR_FROM_CONFIG to the virErrorDomain enum. Both of these files 
must be patched in unison to prevent compilation failures.

Signed-off-by: Adam Walters 
---
 include/libvirt/virterror.h | 2 ++
 src/util/virerror.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index e31e9c4..018c880 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -121,6 +121,8 @@ typedef enum {
 VIR_FROM_ACCESS = 55,   /* Error from access control manager */
 VIR_FROM_SYSTEMD = 56,  /* Error from systemd code */
 
+VIR_FROM_CONFIG = 57,  /* Error from config driver */
+
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_DOMAIN_LAST
 # endif
diff --git a/src/util/virerror.c b/src/util/virerror.c
index d9a9fc4..04a3acf 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -124,6 +124,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
 
   "Access Manager", /* 55 */
   "Systemd",
+
+  "Config Driver", /* 57 */
 )
 
 
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv6 1/3] qemu: conf: Implement qemuAddRBDPoolSourceHost function

[Adam Walters]

This function is a helper function that grabs RBD hosts from a storage pool 
definition, and applies them to the domain's disk defi
nitions at runtime. This is a pre-requisite for RBD storage pool support in the 
domain XML.

Signed-off-by: Adam Walters 
---
 src/qemu/qemu_conf.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 4378791..2d42c3b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1247,6 +1247,45 @@ cleanup:
 }
 
 static int
+qemuAddRBDPoolSourceHost(virDomainDiskDefPtr def,
+   virStoragePoolDefPtr pooldef)
+{
+int ret = -1;
+size_t i = 0;
+char **tokens = NULL;
+
+def->nhosts = pooldef->source.nhost;
+
+if (VIR_ALLOC_N(def->hosts, def->nhosts) < 0)
+goto cleanup;
+
+for (i = 0; i < def->nhosts; i++) {
+if (VIR_STRDUP(def->hosts[i].name, pooldef->source.hosts[i].name) < 0)
+goto cleanup;
+
+if (virAsprintf(&def->hosts[i].port, "%d",
+pooldef->source.hosts[i].port ?
+pooldef->source.hosts[i].port :
+6789) < 0)
+goto cleanup;
+
+/* Storage pools have not supported these 2 attributes yet,
+ * use the defaults.
+ */
+def->hosts[i].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP;
+def->hosts[i].socket = NULL;
+}
+
+def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
+
+ret = 0;
+
+cleanup:
+virStringFreeList(tokens);
+return ret;
+}
+
+static int
 qemuTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def,
 virStoragePoolDefPtr pooldef)
 {
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv6 0/3] Implement RBD storage pool support

[Adam Walters]

Here is a re-based re-submission of my patches to implement RBD storage pool 
support for QEMU domains. Nothing in it has changed other than it has been 
rebased against the latest. The race condition I located still exists, but I 
have some patches forthcoming to address that issue. The code here still works 
well for me, but I only have the one host to test this on.

If possible, I would like to try and get this merged for the 1.2.1 release 
cycle, so that other users can benefit from the addition of added storage pool 
support. As always, if you find any problems with my patches, please let me 
know, and I will fix them as soon as I can.

Adam Walters (3):
  qemu: conf: Implement qemuAddRBDPoolSourceHost function
  qemu: conf: Implement RBD storage pool support
  domain: conf: Fix secret type checking

 src/conf/domain_conf.c |  6 +++--
 src/qemu/qemu_conf.c   | 62 +-
 2 files changed, 65 insertions(+), 3 deletions(-)

-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv6 3/3] domain: conf: Fix secret type checking

[Adam Walters]

This patch fixes the secret type checking done in the virDomainDiskDefParseXML 
function. Previously, it would not allow any volumes that utilized a secret. 
This patch is a simple bypass of the checking code for volumes.

Signed-off-by: Adam Walters 
---
 src/conf/domain_conf.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0079234..80537cd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5369,7 +5369,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 cur = cur->next;
 }
 
-if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage) 
{
+if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage  
&&
+def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("invalid secret type '%s'"),
virSecretUsageTypeTypeToString(auth_secret_usage));
@@ -18107,7 +18108,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
 if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK ||
 (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME &&
  disk->srcpool &&
- disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT))
+ (disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT ||
+  disk->srcpool->actualtype == VIR_DOMAIN_DISK_TYPE_NETWORK)))
 return 0;
 
 if (iter(disk, disk->src, 0, opaque) < 0)
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv6 2/3] qemu: conf: Implement RBD storage pool support

[Adam Walters]

This patch modifies the qemuTranslateDiskSourcePool function to add RBD storage 
pool support. The code is heavily based off of the existing iSCSI code, but 
modified for RBD support. The modification calls the qemuAddRBDPoolSourceHost 
from my previous patch, along with setting up the ceph user and secret, if 
applicable.

Signed-off-by: Adam Walters 
---
 src/qemu/qemu_conf.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2d42c3b..c28908a 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1478,8 +1478,29 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
}
break;
 
-case VIR_STORAGE_POOL_MPATH:
 case VIR_STORAGE_POOL_RBD:
+if (def->startupPolicy) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("'startupPolicy' is only valid for "
+ "'file' type volume"));
+goto cleanup;
+}
+
+def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK;
+def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
+
+if (!(def->src = virStorageVolGetPath(vol)))
+goto cleanup;
+
+if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0)
+goto cleanup;
+
+if (qemuAddRBDPoolSourceHost(def, pooldef) < 0)
+goto cleanup;
+
+break;
+
+case VIR_STORAGE_POOL_MPATH:
 case VIR_STORAGE_POOL_SHEEPDOG:
 case VIR_STORAGE_POOL_GLUSTER:
 case VIR_STORAGE_POOL_LAST:
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/5] qemu: Fix job usage in qemuDomainBlockCopy

[Jiri Denemark]

Every API that is going to begin a job should do that before fetching
data from vm->def.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c | 25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dad8450..2fb643a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14427,7 +14427,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
 virQEMUDriverPtr driver = conn->privateData;
 qemuDomainObjPrivatePtr priv;
 char *device = NULL;
-virDomainDiskDefPtr disk;
+virDomainDiskDefPtr disk = NULL;
 int ret = -1;
 int idx;
 struct stat st;
@@ -14442,29 +14442,32 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
 priv = vm->privateData;
 cfg = virQEMUDriverGetConfig(driver);
 
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("domain is not running"));
-goto cleanup;
+goto endjob;
 }
 
 device = qemuDiskPathToAlias(vm, path, &idx);
 if (!device) {
-goto cleanup;
+goto endjob;
 }
 disk = vm->def->disks[idx];
 if (disk->mirror) {
 virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
_("disk '%s' already in active block copy job"),
disk->dst);
-goto cleanup;
+goto endjob;
 }
 
 if (!(virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_MIRROR) &&
   virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC))) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("block copy is not supported with this QEMU binary"));
-goto cleanup;
+goto endjob;
 }
 if (vm->persistent) {
 /* XXX if qemu ever lets us start a new domain with mirroring
@@ -14473,17 +14476,9 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
  * this on persistent domains.  */
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("domain is not transient"));
-goto cleanup;
-}
-
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
-goto cleanup;
-
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("domain is not running"));
 goto endjob;
 }
+
 if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
 goto endjob;
 
@@ -14573,7 +14568,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
 endjob:
 if (need_unlink && unlink(dest))
 VIR_WARN("unable to unlink just-created %s", dest);
-if (ret < 0)
+if (ret < 0 && disk)
 disk->mirrorFormat = VIR_STORAGE_FILE_NONE;
 VIR_FREE(mirror);
 if (!qemuDomainObjEndJob(driver, vm))
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/5] qemu: Fix job usage in qemuDomainBlockJobImpl

[Jiri Denemark]

Every API that is going to begin a job should do that before fetching
data from vm->def.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9db0c49..dad8450 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14241,16 +14241,25 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 goto cleanup;
 }
 
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("domain is not running"));
+goto endjob;
+}
+
 device = qemuDiskPathToAlias(vm, path, &idx);
 if (!device)
-goto cleanup;
+goto endjob;
 disk = vm->def->disks[idx];
 
 if (mode == BLOCK_JOB_PULL && disk->mirror) {
 virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
_("disk '%s' already in active block copy job"),
disk->dst);
-goto cleanup;
+goto endjob;
 }
 if (mode == BLOCK_JOB_ABORT &&
 (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) &&
@@ -14258,15 +14267,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
 virReportError(VIR_ERR_OPERATION_INVALID,
_("pivot of disk '%s' requires an active copy job"),
disk->dst);
-goto cleanup;
-}
-
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
-goto cleanup;
-
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("domain is not running"));
 goto endjob;
 }
 
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/5] qemu: Fix job usage in several APIs

[Jiri Denemark]

When fixing https://bugzilla.redhat.com/show_bug.cgi?id=1043069 I
realized qemuDomainBlockStats is not the only API that does not acquire
a job early enough. Generally, every API that is going to begin a job
should do that before fetching data from vm->def. The following 5 APIs
failed to do so and moreover used the data fetched early from vm->def
after starting a job. In some circumstances this can lead to a crash.

Jiri Denemark (5):
  qemu: Do not access stale data in virDomainBlockStats
  qemu: Avoid using stale data in virDomainGetBlockInfo
  qemu: Fix job usage in qemuDomainBlockJobImpl
  qemu: Fix job usage in qemuDomainBlockCopy
  qemu: Fix job usage in virDomainGetBlockIoTune

 src/qemu/qemu_driver.c | 92 --
 1 file changed, 44 insertions(+), 48 deletions(-)

-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/5] qemu: Avoid using stale data in virDomainGetBlockInfo

[Jiri Denemark]

Generally, every API that is going to begin a job should do that before
fetching data from vm->def. However, qemuDomainGetBlockInfo does not
know whether it will have to start a job or not before checking vm->def.
To avoid using disk alias that might have been freed while we were
waiting for a job, we use its copy. In case the disk was removed in the
meantime, we will fail with "cannot find statistics for device '...'"
error message.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8823187..9db0c49 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9780,10 +9780,12 @@ cleanup:
 }
 
 
-static int qemuDomainGetBlockInfo(virDomainPtr dom,
-  const char *path,
-  virDomainBlockInfoPtr info,
-  unsigned int flags) {
+static int
+qemuDomainGetBlockInfo(virDomainPtr dom,
+   const char *path,
+   virDomainBlockInfoPtr info,
+   unsigned int flags)
+{
 virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
 int ret = -1;
@@ -9795,6 +9797,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
 int idx;
 int format;
 virQEMUDriverConfigPtr cfg = NULL;
+char *alias = NULL;
 
 virCheckFlags(0, -1);
 
@@ -9901,13 +9904,16 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
 virDomainObjIsActive(vm)) {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 
+if (VIR_STRDUP(alias, disk->info.alias) < 0)
+goto cleanup;
+
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
 goto cleanup;
 
 if (virDomainObjIsActive(vm)) {
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorGetBlockExtent(priv->mon,
-disk->info.alias,
+alias,
 &info->allocation);
 qemuDomainObjExitMonitor(driver, vm);
 } else {
@@ -9921,6 +9927,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
 }
 
 cleanup:
+VIR_FREE(alias);
 virStorageFileFreeMetadata(meta);
 VIR_FORCE_CLOSE(fd);
 if (vm)
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 5/5] qemu: Fix job usage in virDomainGetBlockIoTune

[Jiri Denemark]

Every API that is going to begin a job should do that before fetching
data from vm->def.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2fb643a..b54aa36 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15056,12 +15056,6 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
 goto cleanup;
 }
 
-device = qemuDiskPathToAlias(vm, disk, NULL);
-
-if (!device) {
-goto cleanup;
-}
-
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
@@ -15069,6 +15063,11 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
 &persistentDef) < 0)
 goto endjob;
 
+device = qemuDiskPathToAlias(vm, disk, NULL);
+if (!device) {
+goto endjob;
+}
+
 if (flags & VIR_DOMAIN_AFFECT_LIVE) {
 priv = vm->privateData;
 qemuDomainObjEnterMonitor(driver, vm);
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/5] qemu: Do not access stale data in virDomainBlockStats

[Jiri Denemark]

https://bugzilla.redhat.com/show_bug.cgi?id=1043069

When virDomainDetachDeviceFlags is called concurrently to
virDomainBlockStats: libvirtd may crash because qemuDomainBlockStats
finds a disk in vm->def before getting a job on a domain and uses the
disk pointer after getting the job. However, the domain in unlocked
while waiting on a job condition and thus data behind the disk pointer
may disappear. This happens when thread 1 runs
virDomainDetachDeviceFlags and enters monitor to actually remove the
disk. Then another thread starts running virDomainBlockStats, finds the
disk in vm->def, and while it's waiting on the job condition (owned by
the first thread), the first thread finishes the disk removal. When the
second thread gets the job, the memory pointed to be the disk pointer is
already gone.

That said, every API that is going to begin a job should do that before
fetching data from vm->def.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 45d11cd..8823187 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9020,34 +9020,29 @@ qemuDomainBlockStats(virDomainPtr dom,
 if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+goto cleanup;
+
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
-goto cleanup;
+goto endjob;
 }
 
 if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
 virReportError(VIR_ERR_INVALID_ARG,
_("invalid path: %s"), path);
-goto cleanup;
+goto endjob;
 }
 disk = vm->def->disks[idx];
 
 if (!disk->info.alias) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("missing disk device alias name for %s"), disk->dst);
-goto cleanup;
+goto endjob;
 }
 
 priv = vm->privateData;
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
-goto cleanup;
-
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
-goto endjob;
-}
 
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorGetBlockStatsInfo(priv->mon,
-- 
1.8.5.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] Implementation deficiency in virInitctlSetRunLevel v4

[Eric Blake]

On 12/20/2013 01:41 PM, Eric Blake wrote:
> On 12/20/2013 09:24 AM, Reco wrote:
>> Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
>> lxcDomainReboot.
>>

>>  static int
>> +virDomainShutdownCallback(pid_t pid ATTRIBUTE_UNUSED,
>> +  void *opaque ATTRIBUTE_UNUSED)
>> +{
>> +int rc;
>> +rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
>> +return rc;
> 
> Hmm.  The callback's return value being used directly as the _exit()
> value in patch 1 is not a good idea; either this function must convert
> -1 to EXIT_FAILURE, or we should fix the framework to turn our normal -1
> conventions into the correct exit code (I'm opting for the latter).

Eww, it's even worse than that.  virInitctlSetRunLevel() is documented
as returning -1 on failure, 0 if initctl doesn't exist, and 1 on
success; but virProcessRunInMountNamespace() was declaring failure on
all but a return status of 0 (which only happens when initctl doesn't
exist), which means failure and success both get lumped into the
callback mechanism declaring failure.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] Implementation deficiency in virInitctlSetRunLevel v4

[Eric Blake]

On 12/20/2013 09:24 AM, Reco wrote:
> Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
> lxcDomainReboot.
> 

This fails to compile, due to [1] below:

  CCLD libvirt_lxc
lxc/lxc_driver.c:2783:1: error: return type defaults to 'int' [-Werror]
 virDomainRebootCallback(pid_t pid ATTRIBUTE_UNUSED,
 ^
lxc/lxc_driver.c:2783:1: error: no previous prototype for
'virDomainRebootCallback' [-Werror=missing-prototypes]
cc1: all warnings being treated as errors

> ---
>  src/lxc/lxc_driver.c |   44 
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 

>  
>  static int
> +virDomainShutdownCallback(pid_t pid ATTRIBUTE_UNUSED,
> +  void *opaque ATTRIBUTE_UNUSED)
> +{
> +int rc;
> +rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
> +return rc;

Hmm.  The callback's return value being used directly as the _exit()
value in patch 1 is not a good idea; either this function must convert
-1 to EXIT_FAILURE, or we should fix the framework to turn our normal -1
conventions into the correct exit code (I'm opting for the latter).
Also, you are now invoking code in between the fork/exec boundary off of
a multithreaded parent, which means we must now audit that all calls in
this code path are async-signal-safe (if not, the child will deadlock if
the fork happened in the parent while some other thread held a lock that
the child wants to obtain).

Alas, virInitctlSetRunLevel() calls virAsprintf() calls malloc(), which
is not async safe :(  [we've gotten away with it in the past; LXC is a
Linux-only technology, and glibc's malloc is relatively robust enough
that I've never seen it fail multithreaded fork/exec setups, whereas I
HAVE seen malloc deadlocks on other systems like Solaris].  It also
calls virReportSystemError() on failure, but in a child process, this
may not get logged the same was as an error message in the parent.  More
robust would be having the child be silent and just use distinct error
codes, then let the parent convert error codes into actual error
messages to be logged.  But I think we can wait until an actual report
of a deadlock before going to that effort.

Aha - you are passing NULL for vroot, which means we could get rid of
the malloc() issue.  In fact, we could get rid of the vroot argument
altogether, since all callers now pass NULL, and since we don't want to
be tempted to use the unsafe construct.

>  if (flags == 0 ||
>  (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
> -if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
> -vroot)) < 0) {
> +rc = virProcessRunInMountNamespace(priv->initpid,
> +   virDomainShutdownCallback,
> +   NULL);

We could use the opaque argument to pass our desired runlevel
constant... [2]

>  
> +
> +virDomainRebootCallback(pid_t pid ATTRIBUTE_UNUSED,
> +void *opaque ATTRIBUTE_UNUSED)

[1] you missed the 'static int' line here.

> +{
> +int rc;
> +rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT, NULL);
> +return rc;
> +}

[2] ... and have only one static callback helper instead of 2, since
they differ only by the constant used.

Good - I have enough cleanup to do that I can feel good about claiming
authorship of the final version (thus bypassing the full name question)
while still giving you credit for the find and initial attempts at the
fix.  v5 coming up soon!

-- 
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] libxl: fix segfault when domain create fail

[Jim Fehlig]

Bamvor Jian Zhang wrote:
> there is a segfault in libxl logging in libxl_ctx_free when domain
> create fail. because the log output handler vmessage is freed by
> xtl_logger_destroy before libxl_ctx_free in virDomainObjListRemove.
> move xtl_logger_destroy after libxl_ctx_free could fix this bug.
>
> Signed-off-by: Bamvor Jian Zhang 
> ---
>  src/libxl/libxl_domain.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 68009db..e72c483 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -354,12 +354,11 @@ libxlDomainObjPrivateDispose(void *obj)
>  libxl_evdisable_domain_death(priv->ctx, priv->deathW);
>  
>  virChrdevFree(priv->devs);
> -
> -xtl_logger_destroy(priv->logger);
> +libxl_ctx_free(priv->ctx);
>  if (priv->logger_file)
>  VIR_FORCE_FCLOSE(priv->logger_file);
>  
> -libxl_ctx_free(priv->ctx);
> +xtl_logger_destroy(priv->logger);
>  }
>   

The logger is created before the ctx in libxlDomainObjPrivateInitCtx(),
and should be destroyed after the ctx.  ACK to your fix, I've pushed it.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Eric Blake]

On 12/20/2013 11:40 AM, Reco wrote:
>> You still haven't answered my query from an earlier version: we prefer
>> to list a legal name in the authorship of git commits (after all,
>> copyleft licenses work _because_ of copyright law, but the law prefers
>> working with full names rather than nicknames).  Is there something that
>> I should list you as rather than just "Reco"?
> 
> As far as I'm concerned, this bug was fixed by (in alphabet order):
> 
> Daniel P. Berrange 
> Eric Blake 
> Michal Privoznik 
> 
> /me merely pushed buttons on the keyboard and spammed this maillist.
> 
> Hopefully this eliminates current and future legal issues.
> 
> And 'Reco' is the alias as good as any.

Fair enough.  Then since I'm rewriting a good chunk of the patch
anyways, I'll reassign authorship to myself and leave a line of credit
to you - giving credit is much looser than claiming authorship.  v5
coming up shortly, and I'll wait for your response on that thread to
ensure that what I did was okay.

-- 
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] PanicCheckABIStability: Need to check for existence

[Eric Blake]

On 12/20/2013 11:10 AM, John Ferlan wrote:
> Commit id '4313fead' added a call to virDomainPanicCheckABIStability()
> which did not check whether the panic device existed before making a call
> to virDomainDeviceInfoCheckABIStability() which ended up segfaulting:
> 

> Signed-off-by: John Ferlan 
> ---
> 

> +++ b/src/conf/domain_conf.c
> @@ -13709,6 +13709,9 @@ static bool
>  virDomainPanicCheckABIStability(virDomainPanicDefPtr src,
>  virDomainPanicDefPtr dst)
>  {
> +if (!src && !dst)
> +return true;
> +
>  return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);

Still crashes.  Minimal reproducer:

virsh save $dom $file
virsh save-image-edit $file
  add (or remove) a  line

This should fail ABI compatibility, not crash libvirtd.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Reco]

> You still haven't answered my query from an earlier version: we prefer
> to list a legal name in the authorship of git commits (after all,
> copyleft licenses work _because_ of copyright law, but the law prefers
> working with full names rather than nicknames).  Is there something that
> I should list you as rather than just "Reco"?

As far as I'm concerned, this bug was fixed by (in alphabet order):

Daniel P. Berrange 
Eric Blake 
Michal Privoznik 

/me merely pushed buttons on the keyboard and spammed this maillist.

Hopefully this eliminates current and future legal issues.

And 'Reco' is the alias as good as any.

Reco

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] libxl: avoid crashing if calling `virsh numatune' on inactive domain

[Jim Fehlig]

Dario Faggioli wrote:
> by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as
> possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose()
> happens, without having initialized the nodemap, and hence crashing after some
> invalid free()-s:
>   

Yikes!  ACK to the fix.  I've pushed it.

Regards,
Jim

>  # ./daemon/libvirtd -v
>  *** Error in `/home/xen/libvirt.git/daemon/.libs/lt-libvirtd': 
> munmap_chunk(): invalid pointer: 0x7fdd42592666 ***
>  === Backtrace: =
>  /lib64/libc.so.6(+0x7bbe7)[0x7fdd3f767be7]
>  /lib64/libxenlight.so.4.3(libxl_bitmap_dispose+0xd)[0x7fdd2c88c045]
>  
> /home/xen/libvirt.git/daemon/.libs/../../src/.libs/libvirt_driver_libxl.so(+0x12d26)[0x7fdd2caccd26]
>  
> /home/xen/libvirt.git/src/.libs/libvirt.so.0(virDomainGetNumaParameters+0x15c)[0x7fdd4247898c]
>  /home/xen/libvirt.git/daemon/.libs/lt-libvirtd(+0x1d9a2)[0x7fdd42ecc9a2]
>  
> /home/xen/libvirt.git/src/.libs/libvirt.so.0(virNetServerProgramDispatch+0x3da)[0x7fdd424e9eaa]
>  /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0x1a6f38)[0x7fdd424e3f38]
>  /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa81e5)[0x7fdd423e51e5]
>  /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa783e)[0x7fdd423e483e]
>  /lib64/libpthread.so.0(+0x7c53)[0x7fdd3febbc53]
>  /lib64/libc.so.6(clone+0x6d)[0x7fdd3f7e1dbd]
>
> Signed-off-by: Dario Faggili 
> Cc: Jim Fehlig 
> Cc: Ian Jackson 
> ---
>  src/libxl/libxl_driver.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 29aa6c7..d91744f 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3958,6 +3958,8 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
>   * the filtering on behalf of older clients that can't parse it. */
>  flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
>  
> +libxl_bitmap_init(&nodemap);
> +
>  if (!(vm = libxlDomObjFromDomain(dom)))
>  goto cleanup;
>  
> @@ -3972,8 +3974,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
>  
>  priv = vm->privateData;
>  
> -libxl_bitmap_init(&nodemap);
> -
>  if ((*nparams) == 0) {
>  *nparams = LIBXL_NUMA_NPARAM;
>  ret = 0;
>
>
>
>   

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Eric Blake]

On 12/20/2013 10:53 AM, Reco wrote:
>> At any rate, since Dan has ack'ed it, and it fixes a CVE (where we're
>> still waiting for the number to be assigned, but the flaw is real), I'll
>> go ahead and push this soon.
> 
> Thank you for the advices, Eric, sorry I didn't followed'em to the
> letter. It wasn't intentional, I just don't have that much experience
> with git.

That's okay - you've already shown great initiative by reporting _and
fixing_ a security bug.  Getting all the details right comes with time
and experience, but we all have to start somewhere.

If you read HACKING, but found things confusing or lacking, let us know
- we'd like to improve it for the next first-time contributor.  If you
didn't read HACKING, then that might explain why it took a few tries to
come up to speed on our conventions.

> 
> It's good to know that these small patches benefited the Libvirt
> project.
> 
> Reco

You still haven't answered my query from an earlier version: we prefer
to list a legal name in the authorship of git commits (after all,
copyleft licenses work _because_ of copyright law, but the law prefers
working with full names rather than nicknames).  Is there something that
I should list you as rather than just "Reco"?

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Eric Blake]

On 12/20/2013 09:34 AM, Daniel P. Berrange wrote:
> On Fri, Dec 20, 2013 at 08:24:41PM +0400, Reco wrote:
>> Implement virProcessRunInMountNamespace, which runs callback of type
>> virProcessNamespaceCallback in a container namespace.
>>
>> Hope it'll nail it this time.

Fails 'make syntax-check':

prohibit_fork_wrappers
src/util/virprocess.c:874:switch (cpid = fork()) {
maint.mk: use virCommand for child processes
make: *** [sc_prohibit_fork_wrappers] Error 1

>> +
>> +switch (cpid = fork()) {
>> +case 0:
>> +if (setns(fd, 0) == -1) {
>> +_exit(-1);

exit(-1) is wrong; the user will see $? of 255 (not -1), and it is not
our typical exit status for failure (where we normally want to be < 128
so that it is not confused with exit due to signal).  Besides, I hate
magic numbers; better would be _exit(EXIT_FAILURE).

>> +default:
>> +if (virProcessWait(cpid, &status) < 0 || status < 0) {

No need to pass &status to virProcessWait() if you are going to insist
on 0 status anyways; better to pass NULL and let virProcessWait do the
validation on your behalf.

>> +virReportSystemError(errno,
>> + _("Callback failed with status %i"),
>> + status);

This overwrites the (better) error message that virProcessWait() already
generated.

>> +ret = 1;

This returns a number > 0 on failure, even though our normal conventions
is to return a negative number on failure.

>> +++ b/src/util/virprocess.h
>> @@ -60,4 +60,10 @@ int virProcessSetNamespaces(size_t nfdlist,
>>  int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);
>>  int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
>>  int virProcessSetMaxFiles(pid_t pid, unsigned int files);
>> +
>> +typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);

Worth documenting that the return value of this callback is passed on to
_exit() (and thus that errors should be EXIT_FAILURE rather than our
typical -1 on failure).

I'll repost a full v5 series with my proposed fixes to these issues in a
separate thread, and wait for a review (since this fixes a CVE, it needs
a second set of eyes).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] PanicCheckABIStability: Need to check for existence

[John Ferlan]

Commit id '4313fead' added a call to virDomainPanicCheckABIStability()
which did not check whether the panic device existed before making a call
to virDomainDeviceInfoCheckABIStability() which ended up segfaulting:

Thread 1 (Thread 0x7f5332837700 (LWP 10964)):
(src=, dst=)
at conf/domain_conf.c:13007
(dst=, src=)
at conf/domain_conf.c:13712
(src=, dst=)
at conf/domain_conf.c:14056
(domain=domain@entry=0x7f5357c0, vm=vm@entry=0x7f5336d0,
 defptr=defptr@entry=0x7f5332836978, snap=snap@entry=0x7f5332836970,
 update_current=update_current@entry=0x7f5332836962, flags=flags@entry=1)
at conf/snapshot_conf.c:1230
(domain=0x7f5357c0, xmlDesc=, flags=1)
at qemu/qemu_driver.c:12719
(domain=domain@entry=0x7f5357c0, xmlDesc=0x7f5381d0
 "\n  snap2\n
 new-desc\n  running\n
 \nsnap1\n  \n
 1387487268\n  def->dom
$2 = {virtType = 2, id = -1, ..
...
  rng = 0x0, panic = 0x0, namespaceData = 0x0,...
...
(gdb) print *def->dom
$3 = {virtType = 2, id = -1, ...
...
  rng = 0x0, panic = 0x0, namespaceData = 0x0,...
...
(gdb)

Signed-off-by: John Ferlan 
---

NOTE: Optionally the call could be changed to:

"if (src->panic && !virDomainPanicCheckABIStability(...)"

I went with what I did following the RNGDefCheckABIStability.

 src/conf/domain_conf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0079234..c86af9a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13709,6 +13709,9 @@ static bool
 virDomainPanicCheckABIStability(virDomainPanicDefPtr src,
 virDomainPanicDefPtr dst)
 {
+if (!src && !dst)
+return true;
+
 return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
 }
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Reco]

 Hi.

On Fri, 20 Dec 2013 10:48:39 -0700
Eric Blake  wrote:

> On 12/20/2013 09:24 AM, Reco wrote:
> > Implement virProcessRunInMountNamespace, which runs callback of type
> > virProcessNamespaceCallback in a container namespace.
> > 
> > Hope it'll nail it this time.
> 
> This comment doesn't fit in the commit message; it's better to put
> review comments...
> 
> > 
> > ---
> 
> ...here, after the '---' separator, so that 'git am' will strip the
> information that was useful to reviewers, but not to overall history.
> For the same reason, your patch should be titled:
> 
> [PATCHv4 1/2] ...
> 
> and not
> 
> [PATCH 1/2] ... v4
> 
> since v4 means nothing in libvirt.git (where we don't see the earlier
> three versions), but only on the mailing list.
> 
> Also, when sending a series, it's better to stick both 1/2 and 2/2 as
> in-reply-to a 0/2 cover letter.
> 
> You can do all that with:
>  git send-email -2 --cover-letter --subject-prefix=PATCHv4
> 
> At any rate, since Dan has ack'ed it, and it fixes a CVE (where we're
> still waiting for the number to be assigned, but the flaw is real), I'll
> go ahead and push this soon.

Thank you for the advices, Eric, sorry I didn't followed'em to the
letter. It wasn't intentional, I just don't have that much experience
with git.

It's good to know that these small patches benefited the Libvirt
project.

Reco

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Eric Blake]

On 12/20/2013 09:24 AM, Reco wrote:
> Implement virProcessRunInMountNamespace, which runs callback of type
> virProcessNamespaceCallback in a container namespace.
> 
> Hope it'll nail it this time.

This comment doesn't fit in the commit message; it's better to put
review comments...

> 
> ---

...here, after the '---' separator, so that 'git am' will strip the
information that was useful to reviewers, but not to overall history.
For the same reason, your patch should be titled:

[PATCHv4 1/2] ...

and not

[PATCH 1/2] ... v4

since v4 means nothing in libvirt.git (where we don't see the earlier
three versions), but only on the mailing list.

Also, when sending a series, it's better to stick both 1/2 and 2/2 as
in-reply-to a 0/2 cover letter.

You can do all that with:
 git send-email -2 --cover-letter --subject-prefix=PATCHv4

At any rate, since Dan has ack'ed it, and it fixes a CVE (where we're
still waiting for the number to be assigned, but the flaw is real), I'll
go ahead and push this soon.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events

[Eric Blake]

On 12/20/2013 09:41 AM, Michal Privoznik wrote:
>> Ooh, just noticed that the timestamp is not part of the event data;
>> probably worth adding another parameter to the callback function to list
>> the event timestamp (as knowing when qemu fired an event may indeed be
>> important to a developer using this interface for debugging).  What type
>> would be best?  Is it okay to tie our public interface to struct
>> timespec (which in turn risks problems if a compile-time switch can move
>> between 32- and 64-bit seconds since Epoch), or should I just open-code
>> it to two parameters: 'long long seconds, int microseconds'?
> 
> Well, in qemu code base it seems like they're using struct timeval; but
> typecasting into int64_t both seconds and microseconds. So I'd say it's
> fine for us to go with ULL seconds, uint microseconds. Although I'm not
> that convinced that we should stick with unsigned, signed type will work
> too. At least for another ~25 years on my RPi :)

I'm going signed LL for seconds (-1 if timestamp is not available), and
uint for micros.  Even if the host uses 32-bit time_t for seconds, it
doesn't hurt us to pass the time as 64-bit; and by specifying it as
explicit LL rather than time_t our header remains independent of host
sizing changes.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events

[Michal Privoznik]

On 19.12.2013 21:36, Eric Blake wrote:
> On 12/19/2013 09:01 AM, Daniel P. Berrange wrote:
> 
 +typedef void (*virConnectDomainQemuMonitorEventCallback)(virConnectPtr 
 conn,
 + virDomainPtr dom,
 + const char 
 *event,
 + const char 
 *details,
 + void *opaque);
>>>
>>>
>>> So for instance on this event:
>>> 2013-12-19 15:55:05.575+: 18630: debug : 
>>> qemuMonitorJSONIOProcessLine:172 : QEMU_MONITOR_RECV_EVENT: 
>>> mon=0x7f8c80008910 event={"timestamp": {"seconds": 1387468505, 
>>> "microseconds": 574652}, "event": "SPICE_INITIALIZED", "data": {"server": 
>>> {"auth": "none", "port": "5900", "family": "ipv4", "host": "127.0.0.1"}, 
>>> "client": {"port": "39285", "family": "ipv4", "channel-type": 4, 
>>> "connection-id": 375558326, "host": "127.0.0.1", "channel-id": 0, "tls": 
>>> false}}}
>>>
>>> the callback will be invoked with:
>>> event="SPICE_INITIALIZED"
>>> details="{"server": {"auth": }}"?
>>
> 
> Ooh, just noticed that the timestamp is not part of the event data;
> probably worth adding another parameter to the callback function to list
> the event timestamp (as knowing when qemu fired an event may indeed be
> important to a developer using this interface for debugging).  What type
> would be best?  Is it okay to tie our public interface to struct
> timespec (which in turn risks problems if a compile-time switch can move
> between 32- and 64-bit seconds since Epoch), or should I just open-code
> it to two parameters: 'long long seconds, int microseconds'?

Well, in qemu code base it seems like they're using struct timeval; but
typecasting into int64_t both seconds and microseconds. So I'd say it's
fine for us to go with ULL seconds, uint microseconds. Although I'm not
that convinced that we should stick with unsigned, signed type will work
too. At least for another ~25 years on my RPi :)

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] Implementation deficiency in virInitctlSetRunLevel v4

[Daniel P. Berrange]

On Fri, Dec 20, 2013 at 08:24:52PM +0400, Reco wrote:
> Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
> lxcDomainReboot.
> 
> ---
>  src/lxc/lxc_driver.c |   44 
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index e5298d1..2385f5b 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -2694,12 +2694,21 @@ lxcConnectListAllDomains(virConnectPtr conn,
>  
>  
>  static int
> +virDomainShutdownCallback(pid_t pid ATTRIBUTE_UNUSED,
> +  void *opaque ATTRIBUTE_UNUSED)
> +{
> +int rc;
> +rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
> +return rc;
> +}
> +
> +
> +static int
>  lxcDomainShutdownFlags(virDomainPtr dom,
> unsigned int flags)
>  {
>  virLXCDomainObjPrivatePtr priv;
>  virDomainObjPtr vm;
> -char *vroot = NULL;
>  int ret = -1;
>  int rc;
>  
> @@ -2726,14 +2735,12 @@ lxcDomainShutdownFlags(virDomainPtr dom,
>  goto cleanup;
>  }
>  
> -if (virAsprintf(&vroot, "/proc/%llu/root",
> -(unsigned long long)priv->initpid) < 0)
> -goto cleanup;
> -
>  if (flags == 0 ||
>  (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
> -if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
> -vroot)) < 0) {
> +rc = virProcessRunInMountNamespace(priv->initpid,
> +   virDomainShutdownCallback,
> +   NULL);
> +if (rc < 0) {
>  goto cleanup;
>  }
>  if (rc == 0 && flags != 0 &&
> @@ -2761,7 +2768,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
>  ret = 0;
>  
>  cleanup:
> -VIR_FREE(vroot);
>  if (vm)
>  virObjectUnlock(vm);
>  return ret;
> @@ -2773,13 +2779,22 @@ lxcDomainShutdown(virDomainPtr dom)
>  return lxcDomainShutdownFlags(dom, 0);
>  }
>  
> +
> +virDomainRebootCallback(pid_t pid ATTRIBUTE_UNUSED,
> +void *opaque ATTRIBUTE_UNUSED)
> +{
> +int rc;
> +rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT, NULL);
> +return rc;
> +}
> +
> +
>  static int
>  lxcDomainReboot(virDomainPtr dom,
>  unsigned int flags)
>  {
>  virLXCDomainObjPrivatePtr priv;
>  virDomainObjPtr vm;
> -char *vroot = NULL;
>  int ret = -1;
>  int rc;
>  
> @@ -2806,14 +2821,12 @@ lxcDomainReboot(virDomainPtr dom,
>  goto cleanup;
>  }
>  
> -if (virAsprintf(&vroot, "/proc/%llu/root",
> -(unsigned long long)priv->initpid) < 0)
> -goto cleanup;
> -
>  if (flags == 0 ||
>  (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
> -if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
> -vroot)) < 0) {
> +rc = virProcessRunInMountNamespace(priv->initpid,
> +   virDomainRebootCallback,
> +   NULL);
> +if (rc < 0) {
>  goto cleanup;
>  }
>  if (rc == 0 && flags != 0 &&
> @@ -2841,7 +2854,6 @@ lxcDomainReboot(virDomainPtr dom,
>  ret = 0;
>  
>  cleanup:
> -VIR_FREE(vroot);
>  if (vm)
>  virObjectUnlock(vm);
>  return ret;

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 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Daniel P. Berrange]

On Fri, Dec 20, 2013 at 08:24:41PM +0400, Reco wrote:
> Implement virProcessRunInMountNamespace, which runs callback of type
> virProcessNamespaceCallback in a container namespace.
> 
> Hope it'll nail it this time.
> 
> ---
>  src/libvirt_private.syms |1 +
>  src/util/virprocess.c|   63 
> ++
>  src/util/virprocess.h|6 +
>  3 files changed, 70 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 2dbb8f8..e210fd0 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1646,6 +1646,7 @@ virProcessGetNamespaces;
>  virProcessGetStartTime;
>  virProcessKill;
>  virProcessKillPainfully;
> +virProcessRunInMountNamespace;
>  virProcessSetAffinity;
>  virProcessSetMaxFiles;
>  virProcessSetMaxMemLock;
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 9fc3207..7bb494e 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -31,6 +31,7 @@
>  # include 
>  #endif
>  #include 
> +#include 
>  
>  #ifdef __FreeBSD__
>  # include 
> @@ -847,3 +848,65 @@ int virProcessGetStartTime(pid_t pid,
>  return 0;
>  }
>  #endif
> +
> +#ifdef HAVE_SETNS
> +int virProcessRunInMountNamespace(pid_t pid,
> +  virProcessNamespaceCallback cb,
> +  void *opaque)
> +{
> +char* path = NULL;
> +int ret = -1;
> +int cpid = -1;
> +int status = -1;
> +int fd = -1;
> +
> +if (virAsprintf(&path, "/proc/%llu/ns/mnt",
> +(unsigned long long)pid) < 0) {
> +goto cleanup;
> +}
> +
> +if ((fd = open(path, O_RDONLY)) < 0) {
> +virReportSystemError(errno, "%s",
> + _("Kernel does not provide mount namespace"));
> +goto cleanup;
> +}
> +
> +switch (cpid = fork()) {
> +case 0:
> +if (setns(fd, 0) == -1) {
> +_exit(-1);
> +}
> +
> +ret = cb(pid, opaque);
> +_exit(ret);
> +break;
> +case -1:
> +virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Fork failed"));
> +goto cleanup;
> +default:
> +if (virProcessWait(cpid, &status) < 0 || status < 0) {
> +virReportSystemError(errno,
> + _("Callback failed with status %i"),
> + status);
> +ret = 1;
> +} else {
> +ret = 0;
> +}
> +}
> +
> +cleanup:
> +VIR_FREE(path);
> +VIR_FORCE_CLOSE(fd);
> +return ret;
> +}
> +#else
> +int virProcessRunInMountNamespace(pid_t pid ATTRIBUTE_UNUSED,
> +  virProcessNamespaceCallback cb 
> ATTRIBUTE_UNUSED,
> +  void *opaque ATTRIBUTE_UNUSED)
> +{
> +virReportSystemError(ENOSYS, "%s",
> + _("Mount namespaces are not available on this 
> platform"));
> +return -1;
> +}
> +#endif
> diff --git a/src/util/virprocess.h b/src/util/virprocess.h
> index 9f77bc5..205abf7 100644
> --- a/src/util/virprocess.h
> +++ b/src/util/virprocess.h
> @@ -60,4 +60,10 @@ int virProcessSetNamespaces(size_t nfdlist,
>  int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);
>  int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
>  int virProcessSetMaxFiles(pid_t pid, unsigned int files);
> +
> +typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);
> +
> +int virProcessRunInMountNamespace(pid_t pid,
> +  virProcessNamespaceCallback cb,
> +  void *opaque);
>  #endif /* __VIR_PROCESS_H__ */

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 1/2] Implementation deficiency in virInitctlSetRunLevel v4

[Reco]

Implement virProcessRunInMountNamespace, which runs callback of type
virProcessNamespaceCallback in a container namespace.

Hope it'll nail it this time.

---
 src/libvirt_private.syms |1 +
 src/util/virprocess.c|   63 ++
 src/util/virprocess.h|6 +
 3 files changed, 70 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2dbb8f8..e210fd0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1646,6 +1646,7 @@ virProcessGetNamespaces;
 virProcessGetStartTime;
 virProcessKill;
 virProcessKillPainfully;
+virProcessRunInMountNamespace;
 virProcessSetAffinity;
 virProcessSetMaxFiles;
 virProcessSetMaxMemLock;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 9fc3207..7bb494e 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -31,6 +31,7 @@
 # include 
 #endif
 #include 
+#include 
 
 #ifdef __FreeBSD__
 # include 
@@ -847,3 +848,65 @@ int virProcessGetStartTime(pid_t pid,
 return 0;
 }
 #endif
+
+#ifdef HAVE_SETNS
+int virProcessRunInMountNamespace(pid_t pid,
+  virProcessNamespaceCallback cb,
+  void *opaque)
+{
+char* path = NULL;
+int ret = -1;
+int cpid = -1;
+int status = -1;
+int fd = -1;
+
+if (virAsprintf(&path, "/proc/%llu/ns/mnt",
+(unsigned long long)pid) < 0) {
+goto cleanup;
+}
+
+if ((fd = open(path, O_RDONLY)) < 0) {
+virReportSystemError(errno, "%s",
+ _("Kernel does not provide mount namespace"));
+goto cleanup;
+}
+
+switch (cpid = fork()) {
+case 0:
+if (setns(fd, 0) == -1) {
+_exit(-1);
+}
+
+ret = cb(pid, opaque);
+_exit(ret);
+break;
+case -1:
+virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Fork failed"));
+goto cleanup;
+default:
+if (virProcessWait(cpid, &status) < 0 || status < 0) {
+virReportSystemError(errno,
+ _("Callback failed with status %i"),
+ status);
+ret = 1;
+} else {
+ret = 0;
+}
+}
+
+cleanup:
+VIR_FREE(path);
+VIR_FORCE_CLOSE(fd);
+return ret;
+}
+#else
+int virProcessRunInMountNamespace(pid_t pid ATTRIBUTE_UNUSED,
+  virProcessNamespaceCallback cb 
ATTRIBUTE_UNUSED,
+  void *opaque ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, "%s",
+ _("Mount namespaces are not available on this 
platform"));
+return -1;
+}
+#endif
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 9f77bc5..205abf7 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -60,4 +60,10 @@ int virProcessSetNamespaces(size_t nfdlist,
 int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);
 int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
 int virProcessSetMaxFiles(pid_t pid, unsigned int files);
+
+typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);
+
+int virProcessRunInMountNamespace(pid_t pid,
+  virProcessNamespaceCallback cb,
+  void *opaque);
 #endif /* __VIR_PROCESS_H__ */
-- 
1.7.10.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] Implementation deficiency in virInitctlSetRunLevel v4

[Reco]

Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
lxcDomainReboot.

---
 src/lxc/lxc_driver.c |   44 
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e5298d1..2385f5b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2694,12 +2694,21 @@ lxcConnectListAllDomains(virConnectPtr conn,
 
 
 static int
+virDomainShutdownCallback(pid_t pid ATTRIBUTE_UNUSED,
+  void *opaque ATTRIBUTE_UNUSED)
+{
+int rc;
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
+return rc;
+}
+
+
+static int
 lxcDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
 {
 virLXCDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
-char *vroot = NULL;
 int ret = -1;
 int rc;
 
@@ -2726,14 +2735,12 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 goto cleanup;
 }
 
-if (virAsprintf(&vroot, "/proc/%llu/root",
-(unsigned long long)priv->initpid) < 0)
-goto cleanup;
-
 if (flags == 0 ||
 (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
-if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
-vroot)) < 0) {
+rc = virProcessRunInMountNamespace(priv->initpid,
+   virDomainShutdownCallback,
+   NULL);
+if (rc < 0) {
 goto cleanup;
 }
 if (rc == 0 && flags != 0 &&
@@ -2761,7 +2768,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 ret = 0;
 
 cleanup:
-VIR_FREE(vroot);
 if (vm)
 virObjectUnlock(vm);
 return ret;
@@ -2773,13 +2779,22 @@ lxcDomainShutdown(virDomainPtr dom)
 return lxcDomainShutdownFlags(dom, 0);
 }
 
+
+virDomainRebootCallback(pid_t pid ATTRIBUTE_UNUSED,
+void *opaque ATTRIBUTE_UNUSED)
+{
+int rc;
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT, NULL);
+return rc;
+}
+
+
 static int
 lxcDomainReboot(virDomainPtr dom,
 unsigned int flags)
 {
 virLXCDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
-char *vroot = NULL;
 int ret = -1;
 int rc;
 
@@ -2806,14 +2821,12 @@ lxcDomainReboot(virDomainPtr dom,
 goto cleanup;
 }
 
-if (virAsprintf(&vroot, "/proc/%llu/root",
-(unsigned long long)priv->initpid) < 0)
-goto cleanup;
-
 if (flags == 0 ||
 (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
-if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
-vroot)) < 0) {
+rc = virProcessRunInMountNamespace(priv->initpid,
+   virDomainRebootCallback,
+   NULL);
+if (rc < 0) {
 goto cleanup;
 }
 if (rc == 0 && flags != 0 &&
@@ -2841,7 +2854,6 @@ lxcDomainReboot(virDomainPtr dom,
 ret = 0;
 
 cleanup:
-VIR_FREE(vroot);
 if (vm)
 virObjectUnlock(vm);
 return ret;
-- 
1.7.10.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] Implementation deficiency in virInitctlSetRunLevel v3

[Daniel P. Berrange]

On Fri, Dec 20, 2013 at 07:42:28PM +0400, Reco wrote:
> Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
> lxcDomainReboot.
> 
> ---
>  src/lxc/lxc_driver.c |   44 
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index e5298d1..7f4acbe 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -2694,12 +2694,21 @@ lxcConnectListAllDomains(virConnectPtr conn,
>  
>  
>  static int
> +virDomainShutdownCallback(pid_t pid ATTRIBUTE_UNUSED,
> +  void* opaque ATTRIBUTE_UNUSED)

Nitpick - the '*' should associate with 'opaque' rather than 'void'

> +{
> +int rc;
> +rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
> +return rc;
> +}
> +
> +
> +static int
>  lxcDomainShutdownFlags(virDomainPtr dom,
> unsigned int flags)
>  {
>  virLXCDomainObjPrivatePtr priv;
>  virDomainObjPtr vm;
> -char *vroot = NULL;
>  int ret = -1;
>  int rc;
>  
> @@ -2726,14 +2735,12 @@ lxcDomainShutdownFlags(virDomainPtr dom,
>  goto cleanup;
>  }
>  
> -if (virAsprintf(&vroot, "/proc/%llu/root",
> -(unsigned long long)priv->initpid) < 0)
> -goto cleanup;
> -
>  if (flags == 0 ||
>  (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
> -if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
> -vroot)) < 0) {
> +rc = virProcessRunInMountNamespace(priv->initpid,
> +   (virProcessNamespaceCallback) 
> virDomainShutdownCallback,

I think you can leave out the (virProcessNamespaceCallback) cast
here, since the function prototype already matches what is
required.

> +   NULL);
> +if (rc < 0) {
>  goto cleanup;
>  }
>  if (rc == 0 && flags != 0 &&
> @@ -2761,7 +2768,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
>  ret = 0;
>  
>  cleanup:
> -VIR_FREE(vroot);
>  if (vm)
>  virObjectUnlock(vm);
>  return ret;
> @@ -2773,13 +2779,22 @@ lxcDomainShutdown(virDomainPtr dom)
>  return lxcDomainShutdownFlags(dom, 0);
>  }
>  
> +
> +virDomainRebootCallback(pid_t pid ATTRIBUTE_UNUSED,
> +void* opaque ATTRIBUTE_UNUSED)

Same nitpick

> +{
> +int rc;
> +rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT, NULL);
> +return rc;
> +}
> +
> +
>  static int
>  lxcDomainReboot(virDomainPtr dom,
>  unsigned int flags)
>  {
>  virLXCDomainObjPrivatePtr priv;
>  virDomainObjPtr vm;
> -char *vroot = NULL;
>  int ret = -1;
>  int rc;
>  
> @@ -2806,14 +2821,12 @@ lxcDomainReboot(virDomainPtr dom,
>  goto cleanup;
>  }
>  
> -if (virAsprintf(&vroot, "/proc/%llu/root",
> -(unsigned long long)priv->initpid) < 0)
> -goto cleanup;
> -
>  if (flags == 0 ||
>  (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
> -if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
> -vroot)) < 0) {
> +rc = virProcessRunInMountNamespace(priv->initpid,
> +   (virProcessNamespaceCallback) 
> virDomainRebootCallback,

And here

> +   NULL);
> +if (rc < 0) {
>  goto cleanup;
>  }
>  if (rc == 0 && flags != 0 &&

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] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v3

[Daniel P. Berrange]

On Fri, Dec 20, 2013 at 07:39:21PM +0400, Reco wrote:
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 9fc3207..2e8535e 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -31,6 +31,7 @@
>  # include 
>  #endif
>  #include 
> +#include 
>  
>  #ifdef __FreeBSD__
>  # include 
> @@ -847,3 +848,65 @@ int virProcessGetStartTime(pid_t pid,
>  return 0;
>  }
>  #endif
> +
> +#ifdef HAVE_SETNS
> +int virProcessRunInMountNamespace(pid_t pid,
> +  virProcessNamespaceCallback cb,
> +  void *opaque)
> +{
> +char* path = NULL;
> +int ret = -1;
> +int cpid = -1;
> +int status = -1;
> +int fd = -1;
> +
> +if (virAsprintf(&path, "/proc/%llu/ns/mnt",
> +(unsigned long long)pid) < 0) {
> +goto cleanup;
> +}
> +
> +if ((fd = open(path, O_RDONLY)) < 0) {
> +virReportSystemError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",

The virReportSystemError method wants to be given 'errno' value here.

> + _("Kernel does not provide mount namespace"));
> +goto cleanup;
> +}
> +
> +switch (cpid = fork()) {
> +case 0:
> +if (setns(fd, 0) == -1) {
> +exit(-1);

I think we need  _exit instead of exit

> +}
> +
> +ret = cb(pid, opaque);
> +exit(ret);

And again here.

> +break;
> +case -1:
> +virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Fork failed"));
> +goto cleanup;
> +default:
> +if (waitpid(cpid, &status, 0) < 0 || status < 0) {
> +virReportSystemError(errno,
> + _("Callback failed with status %i"),
> + status);
> +ret = 1;
> +} else {
> +ret = 0;
> +}

You'll want to use virProcessWait(cpiud, &status) here instead
since to avoid tedious waitpid calling conventions

> +}
> +
> +cleanup:
> +VIR_FREE(path);
> +VIR_FORCE_CLOSE(fd);
> +return ret;
> +}
> +#else
> +int virProcessRunInMountNamespace(pid_t pid ATTRIBUTE_UNUSED,
> +  virProcessNamespaceCallback cb 
> ATTRIBUTE_UNUSED,
> +  void *opaque ATTRIBUTE_UNUSED)
> +{
> +virReportSystemError(ENOSYS, "%s",
> + _("Mount namespaces are not available on this 
> platform"));
> +return -1;
> +}
> +#endif


Looks good aside from those small tweaks.

BTW, thanks very much for taking the time to write these patches for us,
as well as reporting the issue in the first place !

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

[libvirt] [PATCH 2/2] Implementation deficiency in virInitctlSetRunLevel v3

[Reco]

Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
lxcDomainReboot.

---
 src/lxc/lxc_driver.c |   44 
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e5298d1..7f4acbe 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2694,12 +2694,21 @@ lxcConnectListAllDomains(virConnectPtr conn,
 
 
 static int
+virDomainShutdownCallback(pid_t pid ATTRIBUTE_UNUSED,
+  void* opaque ATTRIBUTE_UNUSED)
+{
+int rc;
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
+return rc;
+}
+
+
+static int
 lxcDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
 {
 virLXCDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
-char *vroot = NULL;
 int ret = -1;
 int rc;
 
@@ -2726,14 +2735,12 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 goto cleanup;
 }
 
-if (virAsprintf(&vroot, "/proc/%llu/root",
-(unsigned long long)priv->initpid) < 0)
-goto cleanup;
-
 if (flags == 0 ||
 (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
-if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
-vroot)) < 0) {
+rc = virProcessRunInMountNamespace(priv->initpid,
+   (virProcessNamespaceCallback) 
virDomainShutdownCallback,
+   NULL);
+if (rc < 0) {
 goto cleanup;
 }
 if (rc == 0 && flags != 0 &&
@@ -2761,7 +2768,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 ret = 0;
 
 cleanup:
-VIR_FREE(vroot);
 if (vm)
 virObjectUnlock(vm);
 return ret;
@@ -2773,13 +2779,22 @@ lxcDomainShutdown(virDomainPtr dom)
 return lxcDomainShutdownFlags(dom, 0);
 }
 
+
+virDomainRebootCallback(pid_t pid ATTRIBUTE_UNUSED,
+void* opaque ATTRIBUTE_UNUSED)
+{
+int rc;
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT, NULL);
+return rc;
+}
+
+
 static int
 lxcDomainReboot(virDomainPtr dom,
 unsigned int flags)
 {
 virLXCDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
-char *vroot = NULL;
 int ret = -1;
 int rc;
 
@@ -2806,14 +2821,12 @@ lxcDomainReboot(virDomainPtr dom,
 goto cleanup;
 }
 
-if (virAsprintf(&vroot, "/proc/%llu/root",
-(unsigned long long)priv->initpid) < 0)
-goto cleanup;
-
 if (flags == 0 ||
 (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
-if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
-vroot)) < 0) {
+rc = virProcessRunInMountNamespace(priv->initpid,
+   (virProcessNamespaceCallback) 
virDomainRebootCallback,
+   NULL);
+if (rc < 0) {
 goto cleanup;
 }
 if (rc == 0 && flags != 0 &&
@@ -2841,7 +2854,6 @@ lxcDomainReboot(virDomainPtr dom,
 ret = 0;
 
 cleanup:
-VIR_FREE(vroot);
 if (vm)
 virObjectUnlock(vm);
 return ret;
-- 
1.7.10.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v3

[Reco]

Implement virProcessRunInMountNamespace, which runs callback of type
virProcessNamespaceCallback in a container namespace.

---
 src/libvirt_private.syms |1 +
 src/util/virprocess.c|   63 ++
 src/util/virprocess.h|6 +
 3 files changed, 70 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2dbb8f8..3f4b350 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1646,6 +1646,7 @@ virProcessGetNamespaces;
 virProcessGetStartTime;
 virProcessKill;
 virProcessKillPainfully;
+virProcessRunInMountNamespace
 virProcessSetAffinity;
 virProcessSetMaxFiles;
 virProcessSetMaxMemLock;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 9fc3207..2e8535e 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -31,6 +31,7 @@
 # include 
 #endif
 #include 
+#include 
 
 #ifdef __FreeBSD__
 # include 
@@ -847,3 +848,65 @@ int virProcessGetStartTime(pid_t pid,
 return 0;
 }
 #endif
+
+#ifdef HAVE_SETNS
+int virProcessRunInMountNamespace(pid_t pid,
+  virProcessNamespaceCallback cb,
+  void *opaque)
+{
+char* path = NULL;
+int ret = -1;
+int cpid = -1;
+int status = -1;
+int fd = -1;
+
+if (virAsprintf(&path, "/proc/%llu/ns/mnt",
+(unsigned long long)pid) < 0) {
+goto cleanup;
+}
+
+if ((fd = open(path, O_RDONLY)) < 0) {
+virReportSystemError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("Kernel does not provide mount namespace"));
+goto cleanup;
+}
+
+switch (cpid = fork()) {
+case 0:
+if (setns(fd, 0) == -1) {
+exit(-1);
+}
+
+ret = cb(pid, opaque);
+exit(ret);
+break;
+case -1:
+virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Fork failed"));
+goto cleanup;
+default:
+if (waitpid(cpid, &status, 0) < 0 || status < 0) {
+virReportSystemError(errno,
+ _("Callback failed with status %i"),
+ status);
+ret = 1;
+} else {
+ret = 0;
+}
+}
+
+cleanup:
+VIR_FREE(path);
+VIR_FORCE_CLOSE(fd);
+return ret;
+}
+#else
+int virProcessRunInMountNamespace(pid_t pid ATTRIBUTE_UNUSED,
+  virProcessNamespaceCallback cb 
ATTRIBUTE_UNUSED,
+  void *opaque ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, "%s",
+ _("Mount namespaces are not available on this 
platform"));
+return -1;
+}
+#endif
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 9f77bc5..205abf7 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -60,4 +60,10 @@ int virProcessSetNamespaces(size_t nfdlist,
 int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);
 int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
 int virProcessSetMaxFiles(pid_t pid, unsigned int files);
+
+typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);
+
+int virProcessRunInMountNamespace(pid_t pid,
+  virProcessNamespaceCallback cb,
+  void *opaque);
 #endif /* __VIR_PROCESS_H__ */
-- 
1.7.10.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] libxl: avoid crashing if calling `virsh numatune' on inactive domain

[Dario Faggioli]

by, in libxlDomainGetNumaParameters(), calling libxl_bitmap_init() as soon as
possible, which avoids getting to 'cleanup:', where libxl_bitmap_dispose()
happens, without having initialized the nodemap, and hence crashing after some
invalid free()-s:

 # ./daemon/libvirtd -v
 *** Error in `/home/xen/libvirt.git/daemon/.libs/lt-libvirtd': munmap_chunk(): 
invalid pointer: 0x7fdd42592666 ***
 === Backtrace: =
 /lib64/libc.so.6(+0x7bbe7)[0x7fdd3f767be7]
 /lib64/libxenlight.so.4.3(libxl_bitmap_dispose+0xd)[0x7fdd2c88c045]
 
/home/xen/libvirt.git/daemon/.libs/../../src/.libs/libvirt_driver_libxl.so(+0x12d26)[0x7fdd2caccd26]
 
/home/xen/libvirt.git/src/.libs/libvirt.so.0(virDomainGetNumaParameters+0x15c)[0x7fdd4247898c]
 /home/xen/libvirt.git/daemon/.libs/lt-libvirtd(+0x1d9a2)[0x7fdd42ecc9a2]
 
/home/xen/libvirt.git/src/.libs/libvirt.so.0(virNetServerProgramDispatch+0x3da)[0x7fdd424e9eaa]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0x1a6f38)[0x7fdd424e3f38]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa81e5)[0x7fdd423e51e5]
 /home/xen/libvirt.git/src/.libs/libvirt.so.0(+0xa783e)[0x7fdd423e483e]
 /lib64/libpthread.so.0(+0x7c53)[0x7fdd3febbc53]
 /lib64/libc.so.6(clone+0x6d)[0x7fdd3f7e1dbd]

Signed-off-by: Dario Faggili 
Cc: Jim Fehlig 
Cc: Ian Jackson 
---
 src/libxl/libxl_driver.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 29aa6c7..d91744f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3958,6 +3958,8 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
  * the filtering on behalf of older clients that can't parse it. */
 flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
 
+libxl_bitmap_init(&nodemap);
+
 if (!(vm = libxlDomObjFromDomain(dom)))
 goto cleanup;
 
@@ -3972,8 +3974,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom,
 
 priv = vm->privateData;
 
-libxl_bitmap_init(&nodemap);
-
 if ((*nparams) == 0) {
 *nparams = LIBXL_NUMA_NPARAM;
 ret = 0;

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 2/3] libxl: implement virDomainPinVcpuFlags

[Dario Faggioli]

And use it to implement libxlDomainPinVcpu(), similarly to what
happens in the QEMU driver. This way, it is possible to both
query and change the vcpu affinity of a persistent but not
running domain.

In face, before this patch, we have:
 # virsh list --all
  IdName   State
 
  5 debian_32  running
  - fedora20_64shut off
 # virsh vcpupin fedora20_64 0 2-4 --current
 error: this function is not supported by the connection driver: 
virDomainPinVcpuFlags

After (same situation as above):
 # virsh vcpupin  fedora20_64 0 2-4 --current
 # virsh vcpupin  fedora20_64 0
 VCPU: CPU Affinity
 --
0: 2-4

Signed-off-by: Dario Faggioli 
Cc: Jim Fehlig 
Cc: Ian Jackson 
---
 src/libxl/libxl_driver.c |   73 --
 1 file changed, 51 insertions(+), 22 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3855509..e3da646 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2358,45 +2358,62 @@ cleanup:
 }
 
 static int
-libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
-   int maplen)
+libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
+unsigned char *cpumap, int maplen,
+unsigned int flags)
 {
 libxlDriverPrivatePtr driver = dom->conn->privateData;
 libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
-libxlDomainObjPrivatePtr priv;
+virDomainDefPtr targetDef = NULL;
 virDomainObjPtr vm;
 int ret = -1;
-libxl_bitmap map;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
 
 if (!(vm = libxlDomObjFromDomain(dom)))
 goto cleanup;
 
-if (virDomainPinVcpuEnsureACL(dom->conn, vm->def) < 0)
+if (virDomainPinVcpuFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
 goto cleanup;
 
-if (!virDomainObjIsActive(vm)) {
+if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("cannot pin vcpus on an inactive domain"));
+   _("domain is inactive"));
 goto cleanup;
 }
 
-priv = vm->privateData;
-
-map.size = maplen;
-map.map = cpumap;
-if (libxl_set_vcpuaffinity(priv->ctx, dom->id, vcpu, &map) != 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Failed to pin vcpu '%d' with libxenlight"), vcpu);
+if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
+&flags, &targetDef) < 0)
 goto cleanup;
+
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+targetDef = vm->def;
+}
+
+/* Make sure coverity knows targetDef is valid at this point. */
+sa_assert(targetDef);
+
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+libxl_bitmap map = { .size = maplen, .map = cpumap };
+libxlDomainObjPrivatePtr priv;
+
+priv = vm->privateData;
+if (libxl_set_vcpuaffinity(priv->ctx, dom->id, vcpu, &map) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to pin vcpu '%d' with libxenlight"),
+   vcpu);
+goto cleanup;
+}
 }
 
-if (!vm->def->cputune.vcpupin) {
-if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0)
+if (!targetDef->cputune.vcpupin) {
+if (VIR_ALLOC(targetDef->cputune.vcpupin) < 0)
 goto cleanup;
-vm->def->cputune.nvcpupin = 0;
+targetDef->cputune.nvcpupin = 0;
 }
-if (virDomainVcpuPinAdd(&vm->def->cputune.vcpupin,
-&vm->def->cputune.nvcpupin,
+if (virDomainVcpuPinAdd(&targetDef->cputune.vcpupin,
+&targetDef->cputune.nvcpupin,
 cpumap,
 maplen,
 vcpu) < 0) {
@@ -2405,11 +2422,14 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, 
unsigned char *cpumap,
 goto cleanup;
 }
 
-if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
-goto cleanup;
-
 ret = 0;
 
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ret = virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm);
+} else if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ret = virDomainSaveConfig(cfg->configDir, targetDef);
+}
+
 cleanup:
 if (vm)
 virObjectUnlock(vm);
@@ -2418,6 +2438,14 @@ cleanup:
 }
 
 static int
+libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
+   int maplen)
+{
+return libxlDomainPinVcpuFlags(dom, vcpu, cpumap, maplen,
+   VIR_DOMAIN_AFFECT_LIVE);
+}
+
+static int
 libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps

[libvirt] [PATCH v2 3/3] libxl: correctly handle affinity reset in virDomainPinVcpu[Flags]

[Dario Faggioli]

By actually removing the  element (from within the
 section) from the XML, rather than jus update it with
a fully set vcpu affinity mask.

Signed-off-by: Dario Faggioli 
Cc: Jim Fehlig 
Cc: Ian Jackson 
---
 src/libxl/libxl_driver.c |   18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8528106..4705870 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2365,6 +2365,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 libxlDriverPrivatePtr driver = dom->conn->privateData;
 libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
 virDomainDefPtr targetDef = NULL;
+virBitmapPtr pcpumap = NULL;
 virDomainObjPtr vm;
 int ret = -1;
 
@@ -2393,6 +2394,10 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 
 sa_assert(targetDef);
 
+pcpumap = virBitmapNewData(cpumap, maplen);
+if (!pcpumap)
+goto cleanup;
+
 if (flags & VIR_DOMAIN_AFFECT_LIVE) {
 libxl_bitmap map = { .size = maplen, .map = cpumap };
 libxlDomainObjPrivatePtr priv;
@@ -2406,6 +2411,17 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 }
 }
 
+/* full bitmap means reset the settings (if any). */
+if (virBitmapIsAllSet(pcpumap)) {
+if (virDomainVcpuPinDel(targetDef, vcpu) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to delete vcpupin xml for vcpu '%d'"),
+   vcpu);
+goto cleanup;
+}
+goto out;
+}
+
 if (!targetDef->cputune.vcpupin) {
 if (VIR_ALLOC(targetDef->cputune.vcpupin) < 0)
 goto cleanup;
@@ -2421,6 +2437,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 goto cleanup;
 }
 
+out:
 ret = 0;
 
 if (flags & VIR_DOMAIN_AFFECT_LIVE) {
@@ -2432,6 +2449,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int 
vcpu,
 cleanup:
 if (vm)
 virObjectUnlock(vm);
+virBitmapFree(pcpumap);
 virObjectUnref(cfg);
 return ret;
 }

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 1/3] libxl: implement virDomainGetVcpuPinInfo

[Dario Faggioli]

So that it is possible to query vcpu related information of
a persistent but not running domain, like it is for the QEMU
driver.

In fact, before this patch, we have:
 # virsh list --all
  IdName   State
 
  5 debian_32  running
  - fedora20_64shut off
 # virsh vcpuinfo fedora20_64
 error: this function is not supported by the connection driver: 
virDomainGetVcpuPinInfo

After (same situation as above, i.e., fedora20_64 not running):
 # virsh vcpuinfo fedora20_64
 VCPU:   0
 CPU:N/A
 State:  N/A
 CPU timeN/A
 CPU Affinity:   

 VCPU:   1
 CPU:N/A
 State:  N/A
 CPU timeN/A
 CPU Affinity:   

Signed-off-by: Dario Faggioli 
Cc: Jim Fehlig 
Cc: Ian Jackson 
---
 src/libxl/libxl_driver.c |   78 ++
 1 file changed, 78 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 29aa6c7..3855509 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2417,6 +2417,83 @@ cleanup:
 return ret;
 }
 
+static int
+libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
+  unsigned char *cpumaps, int maplen,
+  unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dom->conn->privateData;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+virDomainObjPtr vm = NULL;
+virDomainDefPtr targetDef = NULL;
+virDomainVcpuPinDefPtr *vcpupin_list;
+virBitmapPtr cpumask = NULL;
+int maxcpu, hostcpus, vcpu, pcpu, n, ret = -1;
+unsigned char *cpumap;
+bool pinned;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetVcpuPinInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
+&flags, &targetDef) < 0)
+goto cleanup;
+
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+targetDef = vm->def;
+}
+
+/* Make sure coverity knows targetDef is valid at this point. */
+sa_assert(targetDef);
+
+/* Clamp to actual number of vcpus */
+if (ncpumaps > targetDef->vcpus)
+ncpumaps = targetDef->vcpus;
+
+/* we use cfg->ctx, as vm->privateData->ctx may be NULL if VM is down. */
+if ((hostcpus = libxl_get_max_cpus(cfg->ctx)) < 0)
+goto cleanup;
+
+maxcpu = maplen * 8;
+if (maxcpu > hostcpus)
+maxcpu = hostcpus;
+
+/* initialize cpumaps */
+memset(cpumaps, 0xff, maplen * ncpumaps);
+if (maxcpu % 8) {
+for (vcpu = 0; vcpu < ncpumaps; vcpu++) {
+cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu);
+cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
+}
+}
+
+/* if vcpupin setting exists, there may be unused pcpus */
+for (n = 0; n < targetDef->cputune.nvcpupin; n++) {
+vcpupin_list = targetDef->cputune.vcpupin;
+vcpu = vcpupin_list[n]->vcpuid;
+cpumask = vcpupin_list[n]->cpumask;
+cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu);
+for (pcpu = 0; pcpu < maxcpu; pcpu++) {
+if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0)
+goto cleanup;
+if (!pinned)
+VIR_UNUSE_CPU(cpumap, pcpu);
+}
+}
+ret = ncpumaps;
+
+cleanup:
+if (vm)
+virObjectUnlock(vm);
+virObjectUnref(cfg);
+return ret;
+}
 
 static int
 libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo,
@@ -4244,6 +4321,7 @@ static virDriver libxlDriver = {
 .domainGetVcpusFlags = libxlDomainGetVcpusFlags, /* 0.9.0 */
 .domainPinVcpu = libxlDomainPinVcpu, /* 0.9.0 */
 .domainGetVcpus = libxlDomainGetVcpus, /* 0.9.0 */
+.domainGetVcpuPinInfo = libxlDomainGetVcpuPinInfo, /* 1.2.1 */
 .domainGetXMLDesc = libxlDomainGetXMLDesc, /* 0.9.0 */
 .connectDomainXMLFromNative = libxlConnectDomainXMLFromNative, /* 0.9.0 */
 .connectDomainXMLToNative = libxlConnectDomainXMLToNative, /* 0.9.0 */

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 0/3] libxl: improve vcpu pinning

[Dario Faggioli]

Hi,

Take two, with the comments from Jim on v1 fixed. Basically, that means I've
added a few comments and removed some non necessary safety checking.

Series available here:

 git://xenbits.xen.org/people/dariof/libvirt.git libxl/VcpuPinX-v2

Regards,
Dario

---

Dario Faggioli (3):
  libxl: implement virDomainGetVcpuPinInfo
  libxl: implement virDomainPinVcpuFlags
  libxl: correctly handle affinity reset in virDomainPinVcpu[Flags]


 src/libxl/libxl_driver.c |  166 --
 1 file changed, 145 insertions(+), 21 deletions(-)

-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 2/4] conf: add support for panic device

[Daniel P. Berrange]

On Fri, Dec 20, 2013 at 07:11:00AM -0700, Eric Blake wrote:
> On 12/20/2013 06:59 AM, John Ferlan wrote:
> > 
> > 
> > On 12/09/2013 04:11 AM, Hu Tao wrote:
> > 
> > <...snip...>
> > 
> >>  
> >> +static bool
> >> +virDomainPanicCheckABIStability(virDomainPanicDefPtr src,
> >> +virDomainPanicDefPtr dst)
> >> +{
> >> +return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
> 
> > 
> > Unlike the virDomainRNGDefCheckABIStability() API, the new 
> > virDomainPanicCheckABIStability()
> > has no checks for !src && !dst
> 
> Yay for automated regression testing catching something!  And sorry that
> I missed this in my initial review.  Yes, adding or removing the panic
> device is an ABI incompatibility.

Indeed. The ABI stability checks are something we really ought to
be able to have our unit tests do in fact. At the very least we
could feed in every single XML file we have in the test suite as
both the src & dst params, to serve as an "identity" test.

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] SECURITY: CVE-2013-6436: libvirtd daemon crash when reading memory tunables for LXC guest in shutoff status

[Daniel P. Berrange]

Libvirt Security Notice
===

   Summary: libvirtd daemon crash when reading memory tunables
for LXC guest in shutoff status
   Reported on: 20131209
  Published on: 20131220
  Fixed on: 20131220
   Reported by: Martin Kletzander 
Patched by: Martin Kletzander 
  See also: CVE-2013-6436

Description
---

The lxcDomainGetMemoryParameters method in the LXC driver did not
check whether the guest being accessed was running or not. When
shutoff there will be no virCgroupPtr instance associated with the
guest. Reading memory tunables involves calling methods with the
virCgroupPtr object as a parameter. This will lead to a crash
accessing a NULL pointer.

Impact
--

A user who has permission to invoke the virDomainGetMemoryParameters
API against the LXC driver will be able to crash the libvirtd
daemon. Access to this API is granted to any user who connects to
the read-only libvirtd UNIX domain socket. If ACLs are active,
access is granted to any user with the 'read' permission on the
'domain' object, which is granted by default to all users. As a
result an unprivileged user will be able to inflict a denial of
service attack on other users of the libvirtd daemon with higher
privilege.

Workaround
--

The impact can be mitigated by blocking access to the read-only
libvirtd UNIX domain socket, with policykit or the 'auth_unix_ro'
parameter in '/etc/libvirt/libvirtd.conf'. If ACLs are active, the
'read' permission should be removed from any untrusted users. This
will not prevent the crash, but will stop unprivileged users from
inflicting the denial of service on higher privileged users.

Affected product


Name: libvirt
  Repository: git://libvirt.org/git/libvirt.git

  Branch: master
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: f8c1cb90213508c4f32549023b0572ed774e48aa

  Branch: v1.0.5-maint
   Broken in: v1.0.5
   Broken in: v1.0.5.1
   Broken in: v1.0.5.2
   Broken in: v1.0.5.3
   Broken in: v1.0.5.4
   Broken in: v1.0.5.5
   Broken in: v1.0.5.6
   Broken in: v1.0.5.7
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 218bd2e8716bcb4c90acf6ecaf879d606b46606b

  Branch: v1.0.6-maint
   Broken in: v1.0.6
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 80d682fd90bb7e97d8670be4cba1fe153438d7a0

  Branch: v1.1.0-maint
   Broken in: v1.1.0
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 30a589bc4731488ca3428515ed57ce5446a83bbd

  Branch: v1.1.1-maint
   Broken in: v1.1.1
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 9a68d1354233f4cfca686655f8021e9477977e6e

  Branch: v1.1.2-maint
   Broken in: v1.1.2
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 79384018480f11ec6f2c2196039e67a9196d3e3a

  Branch: v1.1.3-maint
   Broken in: v1.1.3
   Broken in: v1.1.3.1
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 66247dc5fffe5b9447f4db377c5adf02e6db97c4

  Branch: v1.1.4-maint
   Broken in: v1.1.4
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 09956c7db764a0958034de6fac58f8e878bf

  Branch: v1.2.0-maint
   Broken in: v1.2.0
   Broken by: cfed9ad4fb28e268e1467a0071c2fbc0c0873969
Fixed by: 705f388bceb4fce21b7c5ebc6310cb467c362239


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 v3 2/4] conf: add support for panic device

[Eric Blake]

On 12/20/2013 06:59 AM, John Ferlan wrote:
> 
> 
> On 12/09/2013 04:11 AM, Hu Tao wrote:
> 
> <...snip...>
> 
>>  
>> +static bool
>> +virDomainPanicCheckABIStability(virDomainPanicDefPtr src,
>> +virDomainPanicDefPtr dst)
>> +{
>> +return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);

> 
> Unlike the virDomainRNGDefCheckABIStability() API, the new 
> virDomainPanicCheckABIStability()
> has no checks for !src && !dst

Yay for automated regression testing catching something!  And sorry that
I missed this in my initial review.  Yes, adding or removing the panic
device is an ABI incompatibility.

> 
> I'll put together a patch for this, but figured I'd ask now if there were
> checks that should also be made in the PanicCheck API...

Not really - a panic device has only two properties: existence, and
address (the existence check is missing, which caused the core; but the
address check is already there).

-- 
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 v3 2/4] conf: add support for panic device

[John Ferlan]

On 12/09/2013 04:11 AM, Hu Tao wrote:

<...snip...>

>  
> +static bool
> +virDomainPanicCheckABIStability(virDomainPanicDefPtr src,
> +virDomainPanicDefPtr dst)
> +{
> +return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info);
> +}
> +


These changes have resulted in a libvirtd crash during the virt-test 
'snapshot_edit' checks:
Thread 1 (Thread 0x7f5332837700 (LWP 10964)):
#0  virDomainDeviceInfoCheckABIStability (src=, dst=) at conf/domain_conf.c:13007
#1  0x7f534211bbc7 in virDomainPanicCheckABIStability (dst=, 
src=) at conf/domain_conf.c:13712
#2  virDomainDefCheckABIStability (src=, dst=) at 
conf/domain_conf.c:14056
#3  0x7f53421340cb in virDomainSnapshotRedefinePrep 
(domain=domain@entry=0x7f5357c0, vm=vm@entry=0x7f5336d0, 
defptr=defptr@entry=0x7f5332836978, snap=snap@entry=0x7f5332836970, 
update_current=update_current@entry=0x7f5332836962, flags=flags@entry=1)
at conf/snapshot_conf.c:1230
#4  0x7f5329e4356c in qemuDomainSnapshotCreateXML (domain=0x7f5357c0, 
xmlDesc=, flags=1) at qemu/qemu_driver.c:12719
#5  0x7f53421a0738 in virDomainSnapshotCreateXML 
(domain=domain@entry=0x7f5357c0, 
xmlDesc=0x7f5381d0 "\n  snap2\n  
new-desc\n  running\n  \n
snap1\n  \n  1387487268\n  
, msg=, ret=0x7f533c80, 
args=0x7f533c40, rerr=0x7f5332836c80, client=) at 
remote_dispatch.h:8223
#7  remoteDispatchDomainSnapshotCreateXMLHelper (server=, 
client=, msg=, rerr=0x7f5332836c80, 
args=0x7f533c40, ret=0x7f533c80) at remote_dispatch.h:8199
#8  0x7f53421ede3a in virNetServerProgramDispatchCall (msg=0x7f5344c1acc0, 
client=0x7f5344c1aa80, server=0x7f5344c01570, prog=0x7f5344c15940)
at rpc/virnetserverprogram.c:435
#9  virNetServerProgramDispatch (prog=0x7f5344c15940, 
server=server@entry=0x7f5344c01570, client=0x7f5344c1aa80, msg=0x7f5344c1acc0)
at rpc/virnetserverprogram.c:305
#10 0x7f53421e7c98 in virNetServerProcessMsg (msg=, 
prog=, client=, srv=0x7f5344c01570)
at rpc/virnetserver.c:165
...

(gdb) up 3

(gdb) print *other->def->dom
$2 = {virtType = 2, id = -1, uuid = 
"\210Γ;\254e@\212\245\034\032h$\230\030\264", name = 0x7f53201bdb60 
"virt-tests-vm1", title = 0x0, 
...
 seclabels = 0x7f53201fe650, watchdog = 0x0, memballoon = 0x7f5320201d10, nvram 
= 0x0, tpm = 0x0, cpu = 0x0, sysinfo = 0x0, redirfilter = 0x0, 
  rng = 0x0, panic = 0x0, namespaceData = 0x0, ns = {parse = 0x7f5329dd2770 
, 
...
(gdb) print *def->dom
$3 = {virtType = 2, id = -1, uuid = 
"\210Γ;\254e@\212\245\034\032h$\230\030\264", name = 0x7f53fdf0 
"virt-tests-vm1", title = 0x0, 
...
  rng = 0x0, panic = 0x0, namespaceData = 0x0, ns = {parse = 0x7f5329dd2770 
, 


Unlike the virDomainRNGDefCheckABIStability() API, the new 
virDomainPanicCheckABIStability()
has no checks for !src && !dst

I'll put together a patch for this, but figured I'd ask now if there were
checks that should also be made in the PanicCheck API...

John


>  
>  /* This compares two configurations and looks for any differences
>   * which will affect the guest ABI. This is primarily to allow
> @@ -13930,6 +13985,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
>  if (!virDomainRNGDefCheckABIStability(src->rng, dst->rng))
>  return false;
>  
> +if (!virDomainPanicCheckABIStability(src->panic, dst->panic))
> +return false;
> +
>  return true;
>  }
>  
> @@ -15776,6 +15834,16 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
>  return 0;
>  }
>  

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] libxl: implement virDomainGetVcpuPinInfo

[Dario Faggioli]

On gio, 2013-12-19 at 21:32 -0700, Jim Fehlig wrote:
> > Signed-off-by: Dario Faggioli 
> > Cc: Jim Fehlig 
> > Cc: Ian Jackson 
> > ---
> >  src/libxl/libxl_driver.c |   83 
> > ++
> >  1 file changed, 83 insertions(+)

> > +static int
> > +libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
> > +  unsigned char *cpumaps, int maplen,
> > +  unsigned int flags)
> > +{
> > +libxlDriverPrivatePtr driver = dom->conn->privateData;
> > +libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> > +virDomainObjPtr vm = NULL;
> > +virDomainDefPtr targetDef = NULL;
> > +virDomainVcpuPinDefPtr *vcpupin_list;
> > +virBitmapPtr cpumask = NULL;
> > +int maxcpu, hostcpus, vcpu, pcpu, n, ret = -1;
> > +unsigned char *cpumap;
> > +bool pinned;
> > +
> > +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> > +  VIR_DOMAIN_AFFECT_CONFIG, -1);
> > +
> > +if (!(vm = libxlDomObjFromDomain(dom)))
> > +goto cleanup;
> > +
> > +if (virDomainGetVcpuPinInfoEnsureACL(dom->conn, vm->def) < 0)
> > +goto cleanup;
> > +
> > +if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
> > +&flags, &targetDef) < 0)
> > +goto cleanup;
> > +
> > +if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> > +targetDef = vm->def;
> > +}
> > +
> > +sa_assert(targetDef);
> >   
> 
> I think we should add a comment stating this is needed to silence
> Coverity, similar to the qemu driver.
> 
Ok, I'll do.

> > +
> > +/* Clamp to actual number of vcpus */
> > +if (ncpumaps > targetDef->vcpus)
> > +ncpumaps = targetDef->vcpus;
> > +
> > +if (!cpumaps || ncpumaps < 1) {
> > +virReportError(VIR_ERR_INVALID_ARG, "%s",
> > +   _("cannot return affinity via a NULL pointer"));
> > +goto cleanup;
> > +}
> >   
> 
> cpumaps is guaranteed to be non-NULL on entry and ncpumaps is guaranteed
> to be > 0 on entry, so the above check can be removed.
> 
Right! I actually did saw that, but then forgot when going down to code
the thing. :-)

I'll get rid of this in v2.

Thanks and Regards,
Dario

-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [BUG] pinvcpu before migration

[Dario Faggioli]

On ven, 2013-12-20 at 09:09 +, Wangyufei (James) wrote:
> hi , I am confused by pinvcpu before migration.
> 
> I have two physical machine. One has 12 CPUs (E5645 2*6 ) as src and the 
> other has 8 CPUs (2*4) as des.
> 
> Step 1 : VM is running on src host (E5645 2*6 CPUs) with its VCPUs pinned on 
> some PCPUs
> 
> Step 2 :Because PCPU count is different between src and des , I should pin 
> VCPUs to all PCPUs .  
> virsh vcpupin vmname 0 0-11 --config --live
> 
> Step 3: Do migrate ,and it failed with error in des libvirt log "Unable to 
> set cpuset.cpus: Invalid argument"
> 
> I find that VM's cpu bitmap on src turns to  
> . vm start fails on des due to its pin info cpuset='0-11' in vm->def.
> 
> In usual case, after I pin VCPU to all PCPU, vcpupin info will be deleted in 
> vm->def.
> 
Is that the case? AFAICT, reset happens upon explicit request, e.g.,
something like:

# virsh vcpupin vmname 0 r --config --live

This is at least what works for me with both QEMU and libxl drivers.

> -- qemuDomainPinVcpuFlags
>   -- if (virBitmapIsAllSet(pcpumap, pCpuCount))
> doReset = true; 
>   -- if (doReset) 
> if (virDomainVcpuPinDel(vm->def, vcpu) < 0)
> 
Exactly, and that virBitmapIsAllSet() is true when you pass 'r' as
cpulist to the `virsh vcpupin' command, rather than some actual set of
pcpus, even if the latter spans all the pcpus on your host.

> I debug libvirt and find that on src which PCPU count is 12 (2*6) 
> virBitmapIsAllSet(pcpumap, pCpuCount) return false
> 
Probably because the bitmap is not 12 bits wide! ;-P

> pcpumap->max_bit is 16.   pcpumap = virBitmapNewData(cpumap, maplen) --> 
> bitmap = virBitmapNew(len * CHAR_BIT);  //here len is 2.
> 
> Although my physical machine only has 12 PCPUs , but here pcpumap->max_bit is 
> len*CHAR_BIT . So bitmap is     ,and it's not ALL SET (ALL 
> SET is    ).
> 
Exactly.

> I tried to fix the bug in this case , but I have not found a good solution.
> 
That's because it's not wrong. I do agree that there may be tricky
cases, but this one of yours doesn't look like one. Just use 'r'.

Regards,
Dario

-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] PCI device assignment bugfixes

[Jiri Denemark]

On Thu, Dec 19, 2013 at 16:53:17 +0200, Laine Stump wrote:
> These were all found as a result of researching Bug 1035490. Patch 1
> fixes the originally reported problem, Patch 2 fixes the cause of
> incorrect error reporting in a later comment of that bug, and Patch 3
> fixes a behavior that I encountered while searching for the cause.
> 
> Laine Stump (3):
>   qemu: properly set MaxMemLock when hotplugging with VFIO
>   qemu: avoid duplicate security label restore on hostdev attach failure
>   qemu: re-add hostdev interfaces to hostdev array on libvirtd restart
> 
>  src/conf/domain_conf.c  |  9 ++---
>  src/qemu/qemu_hotplug.c | 22 +++---
>  2 files changed, 17 insertions(+), 14 deletions(-)

ACK series.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virnettlscontexttest fails with GNUTLS 3.0.28

[Eric Blake]

On 12/20/2013 03:16 AM, Daniel P. Berrange wrote:

>> What distro were you on when you hit this failure?  I'm a little bit
>> reluctant to bump the minimum requirement without knowing a bit more
>> about how common 3.0 is in practice.  Adding more details in your commit
>> log about why you needed it (not just what you changed) makes it easier
>> to review.
> 
> We discussed this on IRC - the earliest version that I tested was
> on Fedora 19 which have GNUTLS 3.1.11 which passes. On OpenSuse
> they have 3.0.28 which failed. Technically we could bisect every
> darn release version between these two to find out where the fix
> came in, but frankly it is easier to just assume 3.1.0 until
> the unlikely event that someone else complains :-)

Yep - updating the commit message to mention OpenSuse, and comparing the
failing 3.0.28 to the working Fedora 3.1.11, is sufficient justification
for bumping to blanket 3.1.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] Fix crash in lxcDomainSetMemoryParameters

[Daniel P. Berrange]

From: Martin Kletzander 

The function doesn't check whether the request is made for active or
inactive domain.  Thus when the domain is not running it still tries
accessing non-existing cgroups (priv->cgroup, which is NULL).

I re-made the function in order for it to work the same way it's qemu
counterpart does.

Reproducer:
 1) Define an LXC domain
 2) Do 'virsh memtune  --hard-limit 133T'

Backtrace:
 Thread 6 (Thread 0x7fffec8c0700 (LWP 26826)):
 #0  0x770edcc4 in virCgroupPathOfController (group=0x0, controller=3,
 key=0x775734bd "memory.limit_in_bytes", path=0x7fffec8bf718) at 
util/vircgroup.c:1764
 #1  0x770e9206 in virCgroupSetValueStr (group=0x0, controller=3,
 key=0x775734bd "memory.limit_in_bytes", value=0x7fffe409f360 
"1073741824")
 at util/vircgroup.c:669
 #2  0x770e98b4 in virCgroupSetValueU64 (group=0x0, controller=3,
 key=0x775734bd "memory.limit_in_bytes", value=1073741824) at 
util/vircgroup.c:740
 #3  0x770ee518 in virCgroupSetMemory (group=0x0, kb=1048576) at 
util/vircgroup.c:1904
 #4  0x770ee675 in virCgroupSetMemoryHardLimit (group=0x0, kb=1048576)
 at util/vircgroup.c:1944
 #5  0x557d54c8 in lxcDomainSetMemoryParameters (dom=0x7fffe40cc420,
 params=0x7fffe409f100, nparams=1, flags=0) at lxc/lxc_driver.c:774
 #6  0x772c20f9 in virDomainSetMemoryParameters (domain=0x7fffe40cc420,
 params=0x7fffe409f100, nparams=1, flags=0) at libvirt.c:4051
 #7  0x5561365f in remoteDispatchDomainSetMemoryParameters 
(server=0x55eb7e00,
 client=0x55ec4b10, msg=0x55eb94e0, rerr=0x7fffec8bfb70, 
args=0x7fffe40b8510)
 at remote_dispatch.h:7621
 #8  0x556133fd in remoteDispatchDomainSetMemoryParametersHelper 
(server=0x55eb7e00,
 client=0x55ec4b10, msg=0x55eb94e0, rerr=0x7fffec8bfb70, 
args=0x7fffe40b8510,
 ret=0x7fffe40b84f0) at remote_dispatch.h:7591
 #9  0x773b293f in virNetServerProgramDispatchCall (prog=0x55ec3ae0,
 server=0x55eb7e00, client=0x55ec4b10, msg=0x55eb94e0)
 at rpc/virnetserverprogram.c:435
 #10 0x773b207f in virNetServerProgramDispatch (prog=0x55ec3ae0,
 server=0x55eb7e00, client=0x55ec4b10, msg=0x55eb94e0)
 at rpc/virnetserverprogram.c:305
 #11 0x773a4d2c in virNetServerProcessMsg (srv=0x55eb7e00, 
client=0x55ec4b10,
 prog=0x55ec3ae0, msg=0x55eb94e0) at rpc/virnetserver.c:165
 #12 0x773a4e8d in virNetServerHandleJob (jobOpaque=0x55ec3e30, 
opaque=0x55eb7e00)
 at rpc/virnetserver.c:186
 #13 0x77187f3f in virThreadPoolWorker (opaque=0x55eb7ac0) at 
util/virthreadpool.c:144
 #14 0x7718733a in virThreadHelper (data=0x55eb7890) at 
util/virthreadpthread.c:161
 #15 0x7468ed89 in start_thread (arg=0x7fffec8c0700) at 
pthread_create.c:308
 #16 0x73da26bd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Signed-off-by: Martin Kletzander 
---
 src/lxc/lxc_driver.c | 112 +++
 1 file changed, 96 insertions(+), 16 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 16258cd..e5298d1 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -742,12 +742,24 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
  int nparams,
  unsigned int flags)
 {
-size_t i;
+virCapsPtr caps = NULL;
+virDomainDefPtr vmdef = NULL;
 virDomainObjPtr vm = NULL;
+virLXCDomainObjPrivatePtr priv = NULL;
+virLXCDriverConfigPtr cfg = NULL;
+virLXCDriverPtr driver = dom->conn->privateData;
+unsigned long long hard_limit;
+unsigned long long soft_limit;
+unsigned long long swap_hard_limit;
+bool set_hard_limit = false;
+bool set_soft_limit = false;
+bool set_swap_hard_limit = false;
+int rc;
 int ret = -1;
-virLXCDomainObjPrivatePtr priv;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
 if (virTypedParamsValidate(params, nparams,
VIR_DOMAIN_MEMORY_HARD_LIMIT,
VIR_TYPED_PARAM_ULLONG,
@@ -762,29 +774,97 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
 goto cleanup;
 
 priv = vm->privateData;
+cfg = virLXCDriverGetConfig(driver);
 
-if (virDomainSetMemoryParametersEnsureACL(dom->conn, vm->def, flags) < 0)
+if (virDomainSetMemoryParametersEnsureACL(dom->conn, vm->def, flags) < 0 ||
+!(caps = virLXCDriverGetCapabilities(driver, false)) ||
+virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
+vm, &flags, &vmdef) < 0)
 goto cleanup;
 
-ret = 0;
-for (i = 0; i < nparams; i++) {
-virTypedParameterPtr param = ¶ms[i];
+if (flags & VIR_DOMAIN_AFFECT_LIVE &&
+!virCgroupHasContr

[libvirt] [PATCH 0/2] Fix crash handling memory parameters in LXC

[Daniel P. Berrange]

The first of these 2 issues is a security issue CVE-2013-6436

Both patches have been pushed to GIT master.

Martin Kletzander (2):
  CVE-2013-6436: fix crash in lxcDomainGetMemoryParameters
  Fix crash in lxcDomainSetMemoryParameters

 src/lxc/lxc_driver.c | 153 +++
 1 file changed, 130 insertions(+), 23 deletions(-)

-- 
1.8.4.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] CVE-2013-6436: fix crash in lxcDomainGetMemoryParameters

[Daniel P. Berrange]

From: Martin Kletzander 

The function doesn't check whether the request is made for active or
inactive domain.  Thus when the domain is not running it still tries
accessing non-existing cgroups (priv->cgroup, which is NULL).

I re-made the function in order for it to work the same way it's qemu
counterpart does.

Reproducer:
 1) Define an LXC domain
 2) Do 'virsh memtune '

Backtrace:
 Thread 6 (Thread 0x7fffec8c0700 (LWP 13387)):
 #0  0x770edcc4 in virCgroupPathOfController (group=0x0, controller=3,
 key=0x775734bd "memory.limit_in_bytes", path=0x7fffec8bf750) at 
util/vircgroup.c:1764
 #1  0x770e958c in virCgroupGetValueStr (group=0x0, controller=3,
 key=0x775734bd "memory.limit_in_bytes", value=0x7fffec8bf7c0) at 
util/vircgroup.c:705
 #2  0x770e9d29 in virCgroupGetValueU64 (group=0x0, controller=3,
 key=0x775734bd "memory.limit_in_bytes", value=0x7fffec8bf810) at 
util/vircgroup.c:804
 #3  0x770ee706 in virCgroupGetMemoryHardLimit (group=0x0, 
kb=0x7fffec8bf8a8)
 at util/vircgroup.c:1962
 #4  0x557d590f in lxcDomainGetMemoryParameters (dom=0x7fffd40024a0,
 params=0x7fffd40027a0, nparams=0x7fffec8bfa24, flags=0) at 
lxc/lxc_driver.c:826
 #5  0x772c28d3 in virDomainGetMemoryParameters (domain=0x7fffd40024a0,
 params=0x7fffd40027a0, nparams=0x7fffec8bfa24, flags=0) at libvirt.c:4137
 #6  0x5563714d in remoteDispatchDomainGetMemoryParameters 
(server=0x55eb7e00,
 client=0x55ebaef0, msg=0x55ebb3e0, rerr=0x7fffec8bfb70, 
args=0x7fffd40024e0,
 ret=0x7fffd4002420) at remote.c:1895
 #7  0x556052c4 in remoteDispatchDomainGetMemoryParametersHelper 
(server=0x55eb7e00,
 client=0x55ebaef0, msg=0x55ebb3e0, rerr=0x7fffec8bfb70, 
args=0x7fffd40024e0,
 ret=0x7fffd4002420) at remote_dispatch.h:4050
 #8  0x773b293f in virNetServerProgramDispatchCall (prog=0x55ec3ae0,
 server=0x55eb7e00, client=0x55ebaef0, msg=0x55ebb3e0)
 at rpc/virnetserverprogram.c:435
 #9  0x773b207f in virNetServerProgramDispatch (prog=0x55ec3ae0,
 server=0x55eb7e00, client=0x55ebaef0, msg=0x55ebb3e0)
 at rpc/virnetserverprogram.c:305
 #10 0x773a4d2c in virNetServerProcessMsg (srv=0x55eb7e00, 
client=0x55ebaef0,
 prog=0x55ec3ae0, msg=0x55ebb3e0) at rpc/virnetserver.c:165
 #11 0x773a4e8d in virNetServerHandleJob (jobOpaque=0x55ebc7e0, 
opaque=0x55eb7e00)
 at rpc/virnetserver.c:186
 #12 0x77187f3f in virThreadPoolWorker (opaque=0x55eb7ac0) at 
util/virthreadpool.c:144
 #13 0x7718733a in virThreadHelper (data=0x55eb7890) at 
util/virthreadpthread.c:161
 #14 0x7468ed89 in start_thread (arg=0x7fffec8c0700) at 
pthread_create.c:308
 #15 0x73da26bd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Signed-off-by: Martin Kletzander 
---
 src/lxc/lxc_driver.c | 41 ++---
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c499182..16258cd 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -794,22 +794,36 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
  int *nparams,
  unsigned int flags)
 {
-size_t i;
+virCapsPtr caps = NULL;
+virDomainDefPtr vmdef = NULL;
 virDomainObjPtr vm = NULL;
+virLXCDomainObjPrivatePtr priv = NULL;
+virLXCDriverPtr driver = dom->conn->privateData;
 unsigned long long val;
 int ret = -1;
-virLXCDomainObjPrivatePtr priv;
+size_t i;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
 
 if (!(vm = lxcDomObjFromDomain(dom)))
 goto cleanup;
 
 priv = vm->privateData;
 
-if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0)
+if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0 ||
+!(caps = virLXCDriverGetCapabilities(driver, false)) ||
+virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
+vm, &flags, &vmdef) < 0)
 goto cleanup;
 
+if (flags & VIR_DOMAIN_AFFECT_LIVE &&
+!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("cgroup memory controller is not mounted"));
+goto cleanup;
+}
+
 if ((*nparams) == 0) {
 /* Current number of memory parameters supported by cgroups */
 *nparams = LXC_NB_MEM_PARAM;
@@ -823,22 +837,34 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
 
 switch (i) {
 case 0: /* fill memory hard limit here */
-if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0)
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+val = vmdef->mem.hard_limit;
+  

Re: [libvirt] [Xen-devel] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver

[Ian Campbell]

On Fri, 2013-12-20 at 11:16 +0100, Stefan Bader wrote:

> > One issue is that xl stored the guest config and then retrieves it for
> > use in xl list -l, but libvirt != xl and therefore has no config file to
> > save.
> 
> Right, that kind of was what I tried to say in many words. :) Oh, hm probably
> with the addition that xl would save configs with one key and libvirt with
> another.

It's not really about the key, libvirt simply does not produce an xl
compatible configuration file (nor should it)

Ian.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Fix explicit usage of default video PCI slots

[Daniel P. Berrange]

On Fri, Dec 20, 2013 at 10:17:22AM +0200, Laine Stump wrote:
> On 12/19/2013 07:41 PM, Ján Tomko wrote:
> > On 12/19/2013 06:18 PM, Jiri Denemark wrote:
> >> On Thu, Dec 19, 2013 at 17:43:33 +0100, Jano Tomko wrote:
> >>> Don't set the PCI address of primary video to the default
> >>> if it's already occupied by another device.
> >>>
> >>> Without this, a primary video card that doesn't have an
> >>> address gets the default one on the 1st pass of PCI address
> >>> allocation, even if it's occupied. This address gets picked
> >>> up by the second pass, and a conflict occurs:
> >>>
> >>> XML error: Attempted double use of PCI slot :00:02.0
> >>> (may need "multifunction='on'" for device on function 0)
> >>>
> >>> Also fix the test that was supposed to catch this.
> >> And does it actually work after this patch? IIRC, the primary video card
> >> is forced to be at :00:02.0 by QEMU. That is, if this address is
> >> occupied by another device, there should be no way to add a video card.
> >>
> > It works without this patch too, if you explicitly specify the PCI address 
> > of
> > the video card (and you have QEMU >= 1.6, because that's when we use -device
> > instead of -vga - (the QEMU_CAPS_DEVICE_VIDEO_PRIMARY capability)).
> 
> Isn't there also something about multi-display support that requires the
> primary video to be at a particular address?

FWIW, the only reason why I had hardcoded display originally was because
we were forced to use -vga instead of -device, and so could't control the
PCI port.

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] virnettlscontexttest fails with GNUTLS 3.0.28

[Daniel P. Berrange]

On Thu, Dec 19, 2013 at 01:42:37PM -0700, Eric Blake wrote:
> On 12/19/2013 09:23 AM, Cédric Bosdonnat wrote:
> > Changed the constraints on gnutls to 3.1+
> > ---
> >  tests/virnettlscontexttest.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c
> > index fc512fc..d9a4e9d 100644
> > --- a/tests/virnettlscontexttest.c
> > +++ b/tests/virnettlscontexttest.c
> > @@ -268,7 +268,8 @@ mymain(void)
> >   * be rejected. GNUTLS < 3 does not reject it and
> >   * we don't anticipate them changing this behaviour
> >   */
> > -DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename, 
> > GNUTLS_VERSION_MAJOR >= 3);
> > +DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename,
> > +GNUTLS_VERSION_MAJOR >= 3 && GNUTLS_VERSION_MINOR >= 1);
> 
> Not quite.  This will reject gnutls 4.0.  It has to be more like:
> 
> (GNUTLS_VERSION_MAJOR == 3 && GNUTLS_VERSION_MINOR >= 1) ||
> GNUTLS_VERSION_MAJOR > 3
> 
> What distro were you on when you hit this failure?  I'm a little bit
> reluctant to bump the minimum requirement without knowing a bit more
> about how common 3.0 is in practice.  Adding more details in your commit
> log about why you needed it (not just what you changed) makes it easier
> to review.

We discussed this on IRC - the earliest version that I tested was
on Fedora 19 which have GNUTLS 3.1.11 which passes. On OpenSuse
they have 3.0.28 which failed. Technically we could bisect every
darn release version between these two to find out where the fix
came in, but frankly it is easier to just assume 3.1.0 until
the unlikely event that someone else complains :-)

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] [Xen-devel] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver

[Stefan Bader]

On 19.12.2013 18:57, Ian Campbell wrote:
> On Thu, 2013-12-19 at 18:06 +0100, Stefan Bader wrote:
>>> How about we:
>>>   * move the init to setdefault to catch the single NIC added via
>>> hotplug case
>>
>> Init of devid?
> 
> Yes, sorry for not being clear.
> 
>>  Hm, would that work as I am not sure there is a simple way of
>> differentiating between a NIC config for a single hotplug and one that is 
>> part
>> of a create-time array...
> 
> The create time array would be initialised with devid's != -1 as part of
> the steps outlined in the following bullets, so setdefault woudln't
> touch it again.
> 
>>
>>>   * we add somewhere early in the domain create path a call to a
>>> function which assigns devids to an entire array of devices (and
>>> do it for all the different device types). Perhaps in
>>> initiate_domain_create() after the calls to
>>> libxl__domain_create_info_setdefault and
>>> libxl__domain_build_info_setdefault but before the loop calling
>>> libxl__device_disk_setdefault for the disks.
>>>   * perhaps that same function should call setdefault too, after
>>> having assigned the device, rather than it being done later in
>>> an adhoc way?
>>>
>>> Does that sound at all plausible?
>>
>> I wonder, well this won't help for any other device types (maybe not really
>> needed), what about just adding the following to the existing loop in
>> domcreate_launch_dm (just a brain dump, not even tried to compile):
>>
>> for (i = 0; i < d_config->num_nics; i++) {
>> /* We have to init the nic here, because we still haven't
>>  * called libxl_device_nic_add at this point, but qemu needs
>>  * the nic information to be complete.
>>  */
>> ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
>> if (ret)
>> goto error_out;
>> +if (d_config->nics[i].devid < 0)
>> +d_config->nics[i].devid = i;
>> }
>>
>> Of course this a gain won't work well if the caller had assigned some devids 
>> but
>> not other.
> 
> Indeed.
> 
>> Ok, maybe do the loop twice, first round sets default and picks the
>> highest pre-assigned devid and second round makes sure any still unassigned 
>> ones
>> are set to ++that.
> 
> That would potentially leave holes, I don't know if that matters. 

Yeah, probably rather rare. This just sounded like a less intrusive change which
would only address the problem that no devid is no option when starting the dm.

> 
>> Oh, just while talking about setdefault. Jim, this is one of the odd things 
>> when
>> moving from xm to xl stack from libvirt: libvirt defaults to the netfront NIC
>> when no model is specified and sets the type. The libxl setdefault function 
>> sets
>> the model to rtl8139 but leaves the type untouched.
> 
> This sounds like a bug in libxl to me -- it should do something
> consistent I think.
> 
>>  So setting no model in the
>> xml config creates a domain with no emulated NIC (this does not matter after
>> Linux is up because the emulated devices get unplugged). Just that PXE boot 
>> will
>> not work. This gets odd because with the old xen (xm) driver, no model meant
>> rtl8139.
>>
>> Sigh, and to hijack this thread even further I noticed a quite unexpected
>> behaviour when starting a domain trhough libvirt and then try to use "xl list
>> -l" to get config details. "xl list" shows all running domains but "xl list 
>> -l"
>> produces something like "you have to specify a domain name". I found the 
>> origin
>> of this to be libxl_userdata_retrieve which takes a userdate_userid as an
>> argument. Libvirt uses "libvirt-xml" for that, while xl uses "xl". This 
>> might be
>> intentional and the bug is just that we need a better check for not finding 
>> the
>> userdata and then skipping those domains. On the other hand ... its after 
>> all in
>> both cases a domain created and started through libxl...
> 
> I think this was discussed a few weeks ago on the list, and there were
> one or two separate bugs and short comings. I'm not sure which subset
> were actually fixed.
> 
> One issue is that xl stored the guest config and then retrieves it for
> use in xl list -l, but libvirt != xl and therefore has no config file to
> save.

Right, that kind of was what I tried to say in many words. :) Oh, hm probably
with the addition that xl would save configs with one key and libvirt with
another. So relying on that at least results in xl not being able to show
details of libvirt created domains (and the other way round).
But as you said, I seem to have missed some discussion about it (which I saw Jim
mentioning a link but have not followed, yet, this morning.

> 
> The solution is probably for the list stuff to be based on dynamically
> gathering the domain info, instead of reparsing the config.

Yes, that sounds good. It would feel like a more intuitive behavior to me (not
that I think I could call myself an

Re: [libvirt] [Xen-devel] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver

[Ian Campbell]

On Thu, 2013-12-19 at 11:39 -0700, Jim Fehlig wrote:
> Stefan Bader wrote:
> > Oh, just while talking about setdefault. Jim, this is one of the odd things 
> > when
> > moving from xm to xl stack from libvirt: libvirt defaults to the netfront 
> > NIC
> > when no model is specified and sets the type. The libxl setdefault function 
> > sets
> > the model to rtl8139 but leaves the type untouched.
> 
> The xend toolstack always creates both emulated and vif devices unless
> 'type=netfront' is explicitly specified.  As you say, the guest gets to
> choose what to do with them.  E.g. PXE boot using the emulated device,
> or have the driver for the PV device unplug the emulated one.  I don't
> think libxl supports this right?

It should do, in fact I thought it was the default.

How are you initialising the libxl_device_nic? Type ==  VIF_IOEMU (which
is the default for a VIF on an HVM guest) means both emulated and pv.
(there were bugs in the semantics here in very early versions of libxl,
but I thought they were fixed even before 4.2)

I don't think there is an option to have just the emulated device --
there is always a PV VIF there even if the guest doesn't use it.

Ian.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/1] virsh nodecpustats erroneously returns stats for offline cpus on Linux

[Pradipta Kumar Banerjee]

[snip]
>>>   for (i = 0; i < *nparams; i++) {
>>>   virNodeCPUStatsPtr param = ¶ms[i];
>>
>> What about this?
>>
>> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
>> index 1838547..aa1ad81 100644
>> --- a/src/nodeinfo.c
>> +++ b/src/nodeinfo.c
>> @@ -637,8 +637,9 @@ linuxNodeGetCPUStats(FILE *procstat,
>>
>>   while (fgets(line, sizeof(line), procstat) != NULL) {
>>   char *buf = line;
>> +char **buf_header = virStringSplit(buf, " ", 2);
>>
>> -if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
>> +if (STREQ(buf_header[0], cpu_header)) { /* aka logical CPU time */
>>   size_t i;
>>
>>   if (sscanf(buf,
>> @@ -697,6 +698,7 @@ linuxNodeGetCPUStats(FILE *procstat,
>>   ret = 0;
>>   goto cleanup;
>>   }
>> +virStringFreeList(buf_header);
>>   }
>>
>>   virReportInvalidArg(cpuNum,
>>
>>
> This is definitely better and lesser amount of code..

I think the version with virStringSplit would need some fine tuning since in its
current form it will not free the memory for the failure cases..Comments ?

Also can some expert here provide some tips on whether in this particular
circumstance it should be fine to allocate/realloc/free memory inside the while
loop.  Would be very helpful..


Thanks,
Pradipta

> 
> Thanks
>>
>>>
>>> -- 
>>> 1.8.3.1
>>>
>>> -- 
>>> libvir-list mailing list
>>> libvir-list@redhat.com
>>> https://www.redhat.com/mailman/listinfo/libvir-list
>>>
>>>
>>>
>>
> 
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [BUG] pinvcpu before migration

[Wangyufei (James)]

hi , I am confused by pinvcpu before migration.

I have two physical machine. One has 12 CPUs (E5645 2*6 ) as src and the other 
has 8 CPUs (2*4) as des.

Step 1 : VM is running on src host (E5645 2*6 CPUs) with its VCPUs pinned on 
some PCPUs

Step 2 :Because PCPU count is different between src and des , I should pin 
VCPUs to all PCPUs .  
virsh vcpupin vmname 0 0-11 --config --live

Step 3: Do migrate ,and it failed with error in des libvirt log "Unable to set 
cpuset.cpus: Invalid argument"

I find that VM's cpu bitmap on src turns to  . 
vm start fails on des due to its pin info cpuset='0-11' in vm->def.

In usual case, after I pin VCPU to all PCPU, vcpupin info will be deleted in 
vm->def.

-- qemuDomainPinVcpuFlags
  -- if (virBitmapIsAllSet(pcpumap, pCpuCount))
doReset = true; 
  -- if (doReset) 
if (virDomainVcpuPinDel(vm->def, vcpu) < 0)


I debug libvirt and find that on src which PCPU count is 12 (2*6) 
virBitmapIsAllSet(pcpumap, pCpuCount) return false

pcpumap->max_bit is 16.   pcpumap = virBitmapNewData(cpumap, maplen) --> bitmap 
= virBitmapNew(len * CHAR_BIT);  //here len is 2.

Although my physical machine only has 12 PCPUs , but here pcpumap->max_bit is 
len*CHAR_BIT . So bitmap is     ,and it's not ALL SET (ALL SET 
is    ).

I tried to fix the bug in this case , but I have not found a good solution.

Any suggestions will be thankful.

Best Regards,
-WangYufei


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]

[cool dharma06]

thanks to all and that is working with libvirt-1.2.0 and xen-4.3.1.

thanks to all people for their valuable suggestions.


regards,
cooldharma06.


On Thu, Dec 19, 2013 at 5:01 PM, Dario Faggioli
wrote:

> [Moving this to libvir, libvir-users in Bcc. Also, added xen-devel]
>
> Jim,
>
> cooldharma06 reports having issues when destroying VMs with libvirt
> 1.2.0 and Xen 4.2.1 (is that the case, cooldharma06)?
>
> Look at the stack trace at the end of this message, or here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1044838
>
> Ian, since it looks like events are involved... Any idea?
>
> cooldharma06, about the Xen 4.2.1, thing, is there a specific reason why
> you use it instead, of, e.g., 4.3.x ? Since you are building from source
> anyway, could you at least try Xen 4.3, to see whether the issue could
> be a bug in 4.2's libxl? I'm asking because, with both Xen 4.3 and the
> curret git tip, I don't have issues destroying domains with virsh.
>
> Thanks and Regards,
> Dario
>
> On gio, 2013-12-19 at 15:42 +0530, cool dharma06 wrote:
> > i did the debugging as you said. Kindly refer the following logs:
> >
> >
> > (gdb) c
> > Continuing.
> > thread apply all bt
> > [New Thread 0x7fc337c6b700 (LWP 29520)]
> >
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x in ?? ()
> > (gdb) thread apply all bt
> >
> >
> > Thread 12 (Thread 0x7fc337c6b700 (LWP 29520)):
> > #0  0x7fc33509f18d in read ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #1  0x7fc32b1555c4 in read_all (fd=26, data=0x7fc3380e0a70,
> > data@entry=0x20, len=len@entry=16, nonblocking=nonblocking@entry=0) at
> > xs.c:365
> > #2  0x7fc32b1556d8 in read_message (h=h@entry=0x7fc324013ee0,
> > nonblocking=nonblocking@entry=0) at xs.c:1071
> > #3  0x7fc32b156005 in read_thread (arg=0x7fc324013ee0) at
> > xs.c:1137
> > #4  0x7fc335097b50 in start_thread ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #5  0x7fc3349daa3d in clone ()
> > from /lib/x86_64-linux-gnu/libc.so.6
> > #6  0x in ?? ()
> >
> >
> > Thread 11 (Thread 0x7fc330e7f700 (LWP 29170)):
> > #0  0x7fc33509c2d4 in pthread_cond_wait@@GLIBC_2.3.2 ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #1  0x7fc3371ae9ea in virCondWait (c=c@entry=0x7fc3380dbd80,
> > m=m@entry=0x7fc3380dbd58) at util/virthreadpthread.c:117
> > #2  0x7fc3371af0cb in virThreadPoolWorker
> > (opaque=opaque@entry=0x7fc3380db870) at util/virthreadpool.c:103
> > #3  0x7fc3371ae686 in virThreadHelper (data=) at
> > util/virthreadpthread.c:161
> > #4  0x7fc335097b50 in start_thread ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #5  0x7fc3349daa3d in clone ()
> > from /lib/x86_64-linux-gnu/libc.so.6
> > #6  0x in ?? ()
> >
> >
> > Thread 10 (Thread 0x7fc33067e700 (LWP 29171)):
> > #0  0x7fc33509c2d4 in pthread_cond_wait@@GLIBC_2.3.2 ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #1  0x7fc3371ae9ea in virCondWait (c=c@entry=0x7fc3380dbd80,
> > m=m@entry=0x7fc3380dbd58) at util/virthreadpthread.c:117
> > #2  0x7fc3371af0cb in virThreadPoolWorker
> > (opaque=opaque@entry=0x7fc3380db640) at util/virthreadpool.c:103
> > #3  0x7fc3371ae686 in virThreadHelper (data=) at
> > util/virthreadpthread.c:161
> > #4  0x7fc335097b50 in start_thread ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #5  0x7fc3349daa3d in clone ()
> > from /lib/x86_64-linux-gnu/libc.so.6
> > #6  0x in ?? ()
> >
> >
> > Thread 9 (Thread 0x7fc32fe7d700 (LWP 29172)):
> > #0  0x7fc33509c2d4 in pthread_cond_wait@@GLIBC_2.3.2 ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #1  0x7fc3371ae9ea in virCondWait (c=c@entry=0x7fc3380dbd80,
> > m=m@entry=0x7fc3380dbd58) at util/virthreadpthread.c:117
> > #2  0x7fc3371af0cb in virThreadPoolWorker
> > (opaque=opaque@entry=0x7fc3380db870) at util/virthreadpool.c:103
> > #3  0x7fc3371ae686 in virThreadHelper (data=) at
> > util/virthreadpthread.c:161
> > #4  0x7fc335097b50 in start_thread ()
> > from /lib/x86_64-linux-gnu/libpthread.so.0
> > #5  0x7fc3349daa3d in clone ()
> > from /lib/x86_64-linux-gnu/libc.so.6
> > #6  0x in ?? ()
> >
> >
> > Thread 8 (Thread 0x7fc32f67c700 (LWP 29173)):
> > #0  0x7fc33497cb9f in realloc ()
> > from /lib/x86_64-linux-gnu/libc.so.6
> > #1  0x7fc3349eec85 in __vasprintf_chk ()
> > from /lib/x86_64-linux-gnu/libc.so.6
> > ---Type  to continue, or q  to quit---
> > #2  0x7fc3371aac0a in vasprintf (__ap=0x7fc32f67b6d0,
> > __fmt=0x7fc337391017 "%llu: %s : %s:%d : %s\n", __ptr=0x7fc32f67b830)
> > at /usr/include/x86_64-linux-gnu/bits/stdio2.h:199
> > #3  virVasprintfInternal (report=report@entry=false, domcode=0,
> > filename=0x0, funcname=0x0, linenr=0, strp=0x7fc32f67b830,
> > fmt=fmt@entry=0x7fc337391017 "%llu: %s : %s:%d : %s\n",
> > list=list@entry=0x7fc32f67b6d0) at util/virstring.c:337
> > #4  0x7fc3371aad1b in virAsprintfInternal
> > (report=re

Re: [libvirt] JNA Error Callback could cause core dump.

[Claudio Bley]

Sorry, I must have taken some bad pills. Just realized that I replied
to a message already more than a year old. I swear my MUA displayed it
as new...

At Thu, 19 Dec 2013 16:55:58 +0100,
Claudio Bley wrote:
> 
> At Thu, 18 Oct 2012 15:48:22 +,
> Benjamin Wang (gendwang) wrote:
-- 
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:

Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Fix explicit usage of default video PCI slots

[Laine Stump]

On 12/19/2013 07:41 PM, Ján Tomko wrote:
> On 12/19/2013 06:18 PM, Jiri Denemark wrote:
>> On Thu, Dec 19, 2013 at 17:43:33 +0100, Jano Tomko wrote:
>>> Don't set the PCI address of primary video to the default
>>> if it's already occupied by another device.
>>>
>>> Without this, a primary video card that doesn't have an
>>> address gets the default one on the 1st pass of PCI address
>>> allocation, even if it's occupied. This address gets picked
>>> up by the second pass, and a conflict occurs:
>>>
>>> XML error: Attempted double use of PCI slot :00:02.0
>>> (may need "multifunction='on'" for device on function 0)
>>>
>>> Also fix the test that was supposed to catch this.
>> And does it actually work after this patch? IIRC, the primary video card
>> is forced to be at :00:02.0 by QEMU. That is, if this address is
>> occupied by another device, there should be no way to add a video card.
>>
> It works without this patch too, if you explicitly specify the PCI address of
> the video card (and you have QEMU >= 1.6, because that's when we use -device
> instead of -vga - (the QEMU_CAPS_DEVICE_VIDEO_PRIMARY capability)).

Isn't there also something about multi-display support that requires the
primary video to be at a particular address?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virnettlscontexttest fails with GNUTLS 3.0.28

[Cedric Bosdonnat]

Hi Eric,

On Thu, 2013-12-19 at 13:42 -0700, Eric Blake wrote:
> > -DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename, 
> > GNUTLS_VERSION_MAJOR >= 3);
> > +DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename,
> > +GNUTLS_VERSION_MAJOR >= 3 && GNUTLS_VERSION_MINOR >= 1);
> 
> Not quite.  This will reject gnutls 4.0.  It has to be more like:
> 
> (GNUTLS_VERSION_MAJOR == 3 && GNUTLS_VERSION_MINOR >= 1) ||
> GNUTLS_VERSION_MAJOR > 3

Oops, indeed I'm wrong here.

> What distro were you on when you hit this failure?  I'm a little bit
> reluctant to bump the minimum requirement without knowing a bit more
> about how common 3.0 is in practice.  Adding more details in your commit
> log about why you needed it (not just what you changed) makes it easier
> to review.

I had the failure on all openSUSE 12.*

--
Cedric

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list