Re: [libvirt] [PATCH] S390: Enhance memballoon handling for virtio-s390
On 01/23/2013 11:21 PM, Eric Blake wrote: ACK and pushed. Thanks Eric. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Non-blocking virStreamRecv always returns -2 (EAGAIN)?
On 23.01.2013 17:41, John Eckersberg wrote:
I'm running the following test program, and it works as written with a
blocking stream. Inside the guest I'm running:
[root@f17-minimal ~]# socat /usr/share/dict/words
/dev/virtio-ports/org.libguestfs.channel.0
As expected on the client side, I get all the words dumped. However if
I swap the virStreamNew lines and instead use the non-blocking stream,
the virStreamRecv call always returns -2.
From http://libvirt.org/internals/rpc.html#apiclientdispatch, I see:
When no thread is performing any RPC method call, or sending stream
data there is still a need to monitor the socket for incoming I/O
related to asynchronous events, or stream data receipt. For this task,
a watch is registered with the event loop which triggers whenever the
socket is readable. This watch is automatically disabled whenever any
other thread grabs the buck, and re-enabled when the buck is released.
If I understand that correctly, shouldn't the watch be responsible for
reading the stream data in this case? Or am I just completely missing
something?
-
#include libvirt.h
int main()
{
virConnectPtr conn;
virDomainPtr dom;
virStreamPtr st;
char buf[1024+1];
int got = 0;
conn = virConnectOpen(qemu+ssh://root@localhost/system);
dom = virDomainLookupByName(conn, f17-minimal);
/* st = virStreamNew(conn, VIR_STREAM_NONBLOCK); */
st = virStreamNew(conn, 0);
virDomainOpenChannel(dom, org.libguestfs.channel.0, st, 0);
while (1) {
got = virStreamRecv(st, buf, 1024);
switch (got) {
case 0:
goto finish;
case -1:
goto free;
case -2:
puts(Retrying);
sleep(1);
continue;
}
buf[got] = '\0';
puts(buf);
}
finish:
virStreamFinish(st);
free:
virStreamFree(st);
return 0;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
In order to use VIR_STREAM_NONBLOCK, you need to call
virStreamEventAddCallback() which registers your stream in libvirtd
event loop so data is sent to you as read from chardev's socket.
However, you need a client event loop then. So the simple program should
look like this [1].
Michal
1: http://pastebin.com/JffxfSmg
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt -- add virtio-scsi disk with scsi-generic
Il 24/01/2013 10:08, 韩诚 ha scritto: Hi, Paolo, All, I read virtio-scsi support proposal v2(http://permalink.gmane.org/gmane.comp.emulators.libvirt/50428). Try to add a virtio-scsi with scsi-generic like this: hostdev type='scsi' source adapter name='scsi_host0'/ address type='scsi' bus='0' target='0' unit='0'/ /source target address type='scsi' controller='0' bus='0' target='0' unit='2'/ /target /hostdev But It turn out to be wrong, showing: error: Failed to define domain from rhel63ga error: XML error: unknown host device source address type 'scsi_host' I'd like to know how shall to add a virtio-scsi disk with scsi-generic option. It's not yet supported by libvirt. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
Currently, there is no reason to hold qemu driver locked
throughout whole API execution. Moreover, we can use the
new qemuDomObjFromDomain() internal API to lookup domain then.
---
src/qemu/qemu_driver.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6d4c1e9..100f10b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2407,19 +2407,12 @@ static int qemuDomainSendKey(virDomainPtr domain,
}
}
-qemuDriverLock(driver);
-vm = virDomainFindByUUID(driver-domains, domain-uuid);
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(domain-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(no domain with matching uuid '%s'), uuidstr);
+if (!(vm = qemuDomObjFromDomain(domain)))
goto cleanup;
-}
priv = vm-privateData;
-if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup;
if (!virDomainObjIsActive(vm)) {
@@ -2428,9 +2421,9 @@ static int qemuDomainSendKey(virDomainPtr domain,
goto endjob;
}
-qemuDomainObjEnterMonitorWithDriver(driver, vm);
+qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSendKey(priv-mon, holdtime, keycodes, nkeycodes);
-qemuDomainObjExitMonitorWithDriver(driver, vm);
+qemuDomainObjExitMonitor(driver, vm);
endjob:
if (qemuDomainObjEndJob(driver, vm) == 0)
@@ -2439,7 +2432,6 @@ endjob:
cleanup:
if (vm)
virObjectUnlock(vm);
-qemuDriverUnlock(driver);
return ret;
}
--
1.8.0.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: Re-add driver unlock to qemuDomainSendKey
On 23.01.2013 14:55, Viktor Mihajlovski wrote: Should have been done in commit 56fd513 already, but was missed due to oversight: qemuDomainSendKey didn't release the driver lock in its cleanup section. This fixes an issue introduced by commit 8c5d2ba. Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com --- V2 Changes: - Removed bogus hunk for qemuDomainManagedSave - Enhanced subject line and patch description src/qemu/qemu_driver.c |1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 72907d2..6d4c1e9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2439,6 +2439,7 @@ endjob: cleanup: if (vm) virObjectUnlock(vm); +qemuDriverUnlock(driver); return ret; } Why do we need qemu driver locked throughout whole API? I don't think we need so. If that's the case, we can simplify domain object lookup by switching to qemuDomObjFromDomain. That is: https://www.redhat.com/archives/libvir-list/2013-January/msg01760.html Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Non-blocking virStreamRecv always returns -2 (EAGAIN)?
On Thu, Jan 24, 2013 at 10:14:33AM +0100, Michal Privoznik wrote:
On 23.01.2013 17:41, John Eckersberg wrote:
I'm running the following test program, and it works as written with a
blocking stream. Inside the guest I'm running:
[root@f17-minimal ~]# socat /usr/share/dict/words
/dev/virtio-ports/org.libguestfs.channel.0
As expected on the client side, I get all the words dumped. However if
I swap the virStreamNew lines and instead use the non-blocking stream,
the virStreamRecv call always returns -2.
From http://libvirt.org/internals/rpc.html#apiclientdispatch, I see:
When no thread is performing any RPC method call, or sending stream
data there is still a need to monitor the socket for incoming I/O
related to asynchronous events, or stream data receipt. For this task,
a watch is registered with the event loop which triggers whenever the
socket is readable. This watch is automatically disabled whenever any
other thread grabs the buck, and re-enabled when the buck is released.
If I understand that correctly, shouldn't the watch be responsible for
reading the stream data in this case? Or am I just completely missing
something?
-
#include libvirt.h
int main()
{
virConnectPtr conn;
virDomainPtr dom;
virStreamPtr st;
char buf[1024+1];
int got = 0;
conn = virConnectOpen(qemu+ssh://root@localhost/system);
dom = virDomainLookupByName(conn, f17-minimal);
/* st = virStreamNew(conn, VIR_STREAM_NONBLOCK); */
st = virStreamNew(conn, 0);
virDomainOpenChannel(dom, org.libguestfs.channel.0, st, 0);
while (1) {
got = virStreamRecv(st, buf, 1024);
switch (got) {
case 0:
goto finish;
case -1:
goto free;
case -2:
puts(Retrying);
sleep(1);
continue;
}
buf[got] = '\0';
puts(buf);
}
finish:
virStreamFinish(st);
free:
virStreamFree(st);
return 0;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
In order to use VIR_STREAM_NONBLOCK, you need to call
virStreamEventAddCallback() which registers your stream in libvirtd
event loop so data is sent to you as read from chardev's socket.
However, you need a client event loop then. So the simple program should
look like this [1].
Hmm, we ough tto have raised an error when he tried to create a stream
with non-blocking flag set, but no event loop registered.
1: http://pastebin.com/JffxfSmg
It is much better to include the code in your mail and not use pastebin
links on the list. When people find this mail in google in 6 months time,
chances are the pastebin link will be dead.
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 2/1] maint: make it easier to sort syms files
On Wed, Jan 23, 2013 at 06:23:56PM -0700, Eric Blake wrote: I got bit by 'make check' complaining that the sort order I got by emacs' sort-lines function differed from expectations. * src/libvirt_private.syms: Add emacs trailer. --- Okay to push this as well? My previous patch mistakenly did this: +++ b/src/libvirt_private.syms @@ -487,10 +487,12 @@ virDomainObjSetState; virDomainObjTaint; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; -virDomainPciRombarModeTypeFromString; -virDomainPciRombarModeTypeToString; virDomainPMStateTypeFromString; virDomainPMStateTypeToString; +virDomainPMSuspendedReasonTypeFromString; +virDomainPMSuspendedReasonTypeToString; +virDomainPciRombarModeTypeFromString; +virDomainPciRombarModeTypeToString; virDomainRedirdevBusTypeFromString; I didn't catch that until 'make check' after I had already posted; I've fixed that sorting locally, but this patch would let me avoid the same problem in the future. src/libvirt_private.syms | 5 + 1 file changed, 5 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1f0e4cf..db2ff62 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1910,3 +1910,8 @@ virXPathUInt; virXPathULong; virXPathULongHex; virXPathULongLong; + +# Let emacs know we want case-insensitive sorting +# Local Variables: +# sort-fold-case: t +# End: -- 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] qemu: don't share kerberos caches between domains
On Wed, Jan 23, 2013 at 06:26:49PM -0700, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=718377 complains that there were some SELinux AVCs when using vnc console over Kerberos. The root problem was that Kerberos tries to set up a cache file, and if we don't tell it where, then all domains use the same cache file, which violates sVirt protections. Setting the environment variable unconditionally should be safe, even for setups where Kerboros won't actually create a cache file. Rare chance for me to point out a typo to Eric instead of the other way around:-P s/Kerboros/Kerberos/ 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] [PATCHv3 0/6] Add additional data to NUMA topology info
Another spin of the series. Difference to previous version: - remove redundant init of some structures - add graceful behavior if topology cannot be discovered - fix incorrect usage of virBitmapParse I'm working on adding more docs for the capabilities XML and the support for the xend driver. Peter Krempa (6): schema: Make the cpuset type reusable across schema files schemas: Add schemas for more CPU topology information in the caps XML conf: Split out NUMA topology formatting to simplify access to data capabilities: Switch CPU data in NUMA topology to a struct capabilities: Add additional data to the NUMA topology info test: Add support for thread and core information for the test driver docs/schemas/basictypes.rng | 6 +++ docs/schemas/capability.rng | 11 + docs/schemas/domaincommon.rng | 5 --- src/conf/capabilities.c | 94 ++- src/conf/capabilities.h | 16 +++- src/libvirt_private.syms | 1 + src/nodeinfo.c| 85 -- src/qemu/qemu_process.c | 2 +- src/test/test_driver.c| 24 +-- src/xen/xend_internal.c | 21 +- 10 files changed, 204 insertions(+), 61 deletions(-) -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/6] schemas: Add schemas for more CPU topology information in the caps XML
This patch adds RNG schemas for adding more information in the topology output of the NUMA section in the capabilities XML. The added elements are designed to provide more information about the placement and topology of the processors in the system to management applications. A demonstration of supported XML added by this patch: capabilities host topology cells num='3' cell id='0' cpus num='4' !-- this is node with Hyperthreading -- cpu id='0' socket_id='0' core_id='0' siblings='0-1'/ cpu id='1' socket_id='0' core_id='0' siblings='0-1'/ cpu id='2' socket_id='0' core_id='1' siblings='2-3'/ cpu id='3' socket_id='0' core_id='1' siblings='2-3'/ /cpus /cell cell id='1' cpus num='4' !-- this is node with modules (Bulldozer) -- cpu id='4' socket_id='0' core_id='2' siblings='4-5'/ cpu id='5' socket_id='0' core_id='3' siblings='4-5'/ cpu id='6' socket_id='0' core_id='4' siblings='6-7'/ cpu id='7' socket_id='0' core_id='5' siblings='6-7'/ /cpus /cell cell id='2' cpus num='4' !-- this is a normal multi-core node -- cpu id='8' socket_id='1' core_id='0' siblings='8'/ cpu id='9' socket_id='1' core_id='1' siblings='9'/ cpu id='10' socket_id='1' core_id='2' siblings='10'/ cpu id='11' socket_id='1' core_id='3' siblings='11'/ /cpus /cell /cells /topology /host /capabilities The socket_id field represents identification of the physical socket the CPU is plugged in. This ID may not be identical to the physical socket ID reported by the kernel. The core_id identifies a core within a socket. Also this field may not accurately represent physical ID's. The core_id is guaranteed to be unique within a cell and a socket. There may be duplicates between sockets. Only cores sharing core_id within one cell and one socket can be considered as threads. Cores sharing core_id within sparate cells are distinct cores. The siblings field is a list of CPU id's the cpu id's the CPU is sibling with - thus a thread. The list is in the cpuset format. --- docs/schemas/capability.rng | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 593d340..53fb04a 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -194,6 +194,17 @@ attribute name='id' ref name='unsignedInt'/ /attribute + optional +attribute name='socket_id' + ref name='unsignedInt'/ +/attribute +attribute name='core_id' + ref name='unsignedInt'/ +/attribute +attribute name='siblings' + ref name='cpuset'/ +/attribute + /optional /element /define -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 1/6] schema: Make the cpuset type reusable across schema files
--- docs/schemas/basictypes.rng | 6 ++ docs/schemas/domaincommon.rng | 5 - 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 38cab16..8e44e8d 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -219,4 +219,10 @@ /data /define + define name=cpuset +data type=string + param name=pattern([0-9]+(-[0-9]+)?|\^[0-9]+)(,([0-9]+(-[0-9]+)?|\^[0-9]+))*/param +/data + /define + /grammar diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7f3320e..1a7d6b5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3684,11 +3684,6 @@ !-- Type library -- - define name=cpuset -data type=string - param name=pattern([0-9]+(-[0-9]+)?|\^[0-9]+)(,([0-9]+(-[0-9]+)?|\^[0-9]+))*/param -/data - /define define name=countCPU data type=unsignedShort param name=pattern[0-9]+/param -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 3/6] conf: Split out NUMA topology formatting to simplify access to data
---
src/conf/capabilities.c | 43 +--
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 365c511..0d2512e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -678,6 +678,28 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps,
return NULL;
}
+static void
+virCapabilitiesFormatNUMATopology(virBufferPtr xml,
+ size_t ncells,
+ virCapsHostNUMACellPtr *cells)
+{
+int i;
+int j;
+
+virBufferAddLit(xml, topology\n);
+virBufferAsprintf(xml, cells num='%zu'\n, ncells);
+for (i = 0; i ncells; i++) {
+virBufferAsprintf(xml, cell id='%d'\n, cells[i]-num);
+virBufferAsprintf(xml, cpus num='%d'\n, cells[i]-ncpus);
+for (j = 0; j cells[i]-ncpus; j++)
+virBufferAsprintf(xml, cpu id='%d'/\n,
+ cells[i]-cpus[j]);
+virBufferAddLit(xml, /cpus\n);
+virBufferAddLit(xml, /cell\n);
+}
+virBufferAddLit(xml, /cells\n);
+virBufferAddLit(xml, /topology\n);
+}
/**
* virCapabilitiesFormatXML:
@@ -752,24 +774,9 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(xml, /migration_features\n);
}
-if (caps-host.nnumaCell) {
-virBufferAddLit(xml, topology\n);
-virBufferAsprintf(xml, cells num='%zu'\n,
- caps-host.nnumaCell);
-for (i = 0 ; i caps-host.nnumaCell ; i++) {
-virBufferAsprintf(xml, cell id='%d'\n,
- caps-host.numaCell[i]-num);
-virBufferAsprintf(xml, cpus num='%d'\n,
- caps-host.numaCell[i]-ncpus);
-for (j = 0 ; j caps-host.numaCell[i]-ncpus ; j++)
-virBufferAsprintf(xml, cpu id='%d'/\n,
- caps-host.numaCell[i]-cpus[j]);
-virBufferAddLit(xml, /cpus\n);
-virBufferAddLit(xml, /cell\n);
-}
-virBufferAddLit(xml, /cells\n);
-virBufferAddLit(xml, /topology\n);
-}
+if (caps-host.nnumaCell)
+virCapabilitiesFormatNUMATopology(xml, caps-host.nnumaCell,
+ caps-host.numaCell);
for (i = 0; i caps-host.nsecModels; i++) {
virBufferAddLit(xml, secmodel\n);
--
1.8.1.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 5/6] capabilities: Add additional data to the NUMA topology info
This patch adds data gathering to the NUMA gathering files and adds
support for outputting the data. The test driver and xend driver need to
be adapted to fill sensible data to the structure in a future patch.
---
src/conf/capabilities.c | 31
src/nodeinfo.c | 76 +
2 files changed, 96 insertions(+), 11 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 575981c..4897b9a 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -688,27 +688,47 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps,
return NULL;
}
-static void
+static int
virCapabilitiesFormatNUMATopology(virBufferPtr xml,
size_t ncells,
virCapsHostNUMACellPtr *cells)
{
int i;
int j;
+char *siblings;
virBufferAddLit(xml, topology\n);
virBufferAsprintf(xml, cells num='%zu'\n, ncells);
for (i = 0; i ncells; i++) {
virBufferAsprintf(xml, cell id='%d'\n, cells[i]-num);
virBufferAsprintf(xml, cpus num='%d'\n, cells[i]-ncpus);
-for (j = 0; j cells[i]-ncpus; j++)
-virBufferAsprintf(xml, cpu id='%d'/\n,
+for (j = 0; j cells[i]-ncpus; j++) {
+virBufferAsprintf(xml, cpu id='%d',
cells[i]-cpus[j].id);
+
+if (cells[i]-cpus[j].siblings) {
+if (!(siblings = virBitmapFormat(cells[i]-cpus[j].siblings)))
{
+virReportOOMError();
+return -1;
+}
+
+virBufferAsprintf(xml,
+ socket_id='%d' core_id='%d' siblings='%s',
+ cells[i]-cpus[j].socket_id,
+ cells[i]-cpus[j].core_id,
+ siblings);
+VIR_FREE(siblings);
+}
+virBufferAddLit(xml, /\n);
+}
+
virBufferAddLit(xml, /cpus\n);
virBufferAddLit(xml, /cell\n);
}
virBufferAddLit(xml, /cells\n);
virBufferAddLit(xml, /topology\n);
+
+return 0;
}
/**
@@ -784,9 +804,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(xml, /migration_features\n);
}
-if (caps-host.nnumaCell)
+if (caps-host.nnumaCell
virCapabilitiesFormatNUMATopology(xml, caps-host.nnumaCell,
- caps-host.numaCell);
+ caps-host.numaCell) 0)
+return NULL;
for (i = 0; i caps-host.nsecModels; i++) {
virBufferAddLit(xml, secmodel\n);
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index c29769c..5ce7d4f 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -79,9 +79,11 @@ freebsdNodeGetCPUCount(void)
#ifdef __linux__
# define CPUINFO_PATH /proc/cpuinfo
# define SYSFS_SYSTEM_PATH /sys/devices/system
+# define SYSFS_CPU_PATH SYSFS_SYSTEM_PATH/cpu
# define PROCSTAT_PATH /proc/stat
# define MEMINFO_PATH /proc/meminfo
# define SYSFS_MEMORY_SHARED_PATH /sys/kernel/mm/ksm
+# define SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX 1024
# define LINUX_NB_CPU_STATS 4
# define LINUX_NB_MEMORY_STATS_ALL 4
@@ -1473,6 +1475,59 @@ cleanup:
# define MASK_CPU_ISSET(mask, cpu) \
(((mask)[((cpu) / n_bits(*(mask)))] ((cpu) % n_bits(*(mask 1)
+static virBitmapPtr
+virNodeGetSiblingsList(const char *dir, int cpu_id)
+{
+char *path = NULL;
+char *buf = NULL;
+virBitmapPtr ret = NULL;
+
+if (virAsprintf(path, %s/cpu%u/topology/thread_siblings_list,
+dir, cpu_id) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX, buf) 0)
+goto cleanup;
+
+if (virBitmapParse(buf, 0, ret, NUMA_MAX_N_CPUS) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Failed to parse thread siblings));
+goto cleanup;
+}
+
+cleanup:
+VIR_FREE(buf);
+VIR_FREE(path);
+return ret;
+}
+
+/* returns 1 on success, 0 if the detection failed and -1 on hard error */
+static int
+virNodeCapsFillCPUInfo(int cpu_id, virCapsHostNUMACellCPUPtr cpu)
+{
+int tmp;
+cpu-id = cpu_id;
+
+if ((tmp = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+ topology/physical_package_id, -1)) 0)
+return 0;
+
+cpu-socket_id = tmp;
+
+if ((tmp = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+ topology/core_id, -1)) 0)
+return 0;
+
+cpu-core_id = tmp;
+
+if (!(cpu-siblings = virNodeGetSiblingsList(SYSFS_CPU_PATH, cpu_id)))
+return -1;
+
+return 0;
+}
+
int
nodeCapsInitNUMA(virCapsPtr caps)
{
@@ -1483,6 +1538,7 @@ nodeCapsInitNUMA(virCapsPtr caps)
int ret = -1;
int
[libvirt] [PATCHv3 4/6] capabilities: Switch CPU data in NUMA topology to a struct
This will allow storing additional topology data in the NUMA topology
definition.
This patch changes the storage type and fixes fallout of the change
across the drivers using it.
This patch also changes semantics of adding new NUMA cell information.
Until now the data were re-allocated and copied to the topology
definition. This patch changes the addition function to steal the
pointer to a pre-allocated structure to simplify the code.
---
src/conf/capabilities.c | 32 +---
src/conf/capabilities.h | 16 ++--
src/libvirt_private.syms | 1 +
src/nodeinfo.c | 15 ++-
src/qemu/qemu_process.c | 2 +-
src/test/test_driver.c | 15 ---
src/xen/xend_internal.c | 21 +++--
7 files changed, 66 insertions(+), 36 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0d2512e..575981c 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -65,12 +65,29 @@ virCapabilitiesNew(virArch hostarch,
return caps;
}
+void
+virCapabilitiesClearHostNUMACellCPUTopology(virCapsHostNUMACellCPUPtr cpus,
+size_t ncpus)
+{
+size_t i;
+
+if (!cpus)
+return;
+
+for (i = 0; i ncpus; i++) {
+virBitmapFree(cpus[i].siblings);
+cpus[i].siblings = NULL;
+}
+}
+
static void
virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell)
{
if (cell == NULL)
return;
+virCapabilitiesClearHostNUMACellCPUTopology(cell-cpus, cell-ncpus);
+
VIR_FREE(cell-cpus);
VIR_FREE(cell);
}
@@ -236,7 +253,7 @@ virCapabilitiesAddHostMigrateTransport(virCapsPtr caps,
* @caps: capabilities to extend
* @num: ID number of NUMA cell
* @ncpus: number of CPUs in cell
- * @cpus: array of CPU ID numbers for cell
+ * @cpus: array of CPU definition structures, the pointer is stolen
*
* Registers a new NUMA cell for a host, passing in a
* array of CPU IDs belonging to the cell
@@ -245,7 +262,7 @@ int
virCapabilitiesAddHostNUMACell(virCapsPtr caps,
int num,
int ncpus,
- const int *cpus)
+ virCapsHostNUMACellCPUPtr cpus)
{
virCapsHostNUMACellPtr cell;
@@ -256,16 +273,9 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps,
if (VIR_ALLOC(cell) 0)
return -1;
-if (VIR_ALLOC_N(cell-cpus, ncpus) 0) {
-VIR_FREE(cell);
-return -1;
-}
-memcpy(cell-cpus,
- cpus,
- ncpus * sizeof(*cpus));
-
cell-ncpus = ncpus;
cell-num = num;
+cell-cpus = cpus;
caps-host.numaCell[caps-host.nnumaCell++] = cell;
@@ -693,7 +703,7 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml,
virBufferAsprintf(xml, cpus num='%d'\n, cells[i]-ncpus);
for (j = 0; j cells[i]-ncpus; j++)
virBufferAsprintf(xml, cpu id='%d'/\n,
- cells[i]-cpus[j]);
+ cells[i]-cpus[j].id);
virBufferAddLit(xml, /cpus\n);
virBufferAddLit(xml, /cell\n);
}
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 19b99c6..f5a5c48 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -84,12 +84,21 @@ struct _virCapsGuest {
virCapsGuestFeaturePtr *features;
};
+typedef struct _virCapsHostNUMACellCPU virCapsHostNUMACellCPU;
+typedef virCapsHostNUMACellCPU *virCapsHostNUMACellCPUPtr;
+struct _virCapsHostNUMACellCPU {
+unsigned int id;
+unsigned int socket_id;
+unsigned int core_id;
+virBitmapPtr siblings;
+};
+
typedef struct _virCapsHostNUMACell virCapsHostNUMACell;
typedef virCapsHostNUMACell *virCapsHostNUMACellPtr;
struct _virCapsHostNUMACell {
int num;
int ncpus;
-int *cpus;
+virCapsHostNUMACellCPUPtr cpus;
};
typedef struct _virCapsHostSecModel virCapsHostSecModel;
@@ -201,7 +210,7 @@ extern int
virCapabilitiesAddHostNUMACell(virCapsPtr caps,
int num,
int ncpus,
- const int *cpus);
+ virCapsHostNUMACellCPUPtr cpus);
extern int
@@ -250,6 +259,9 @@ virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps,
const char *ostype,
virArch arch);
+void
+virCapabilitiesClearHostNUMACellCPUTopology(virCapsHostNUMACellCPUPtr cpu,
+size_t ncpus);
extern virArch
virCapabilitiesDefaultGuestArch(virCapsPtr caps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fc23adc..42eae3e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -56,6 +56,7 @@ virCapabilitiesAddHostFeature;
virCapabilitiesAddHostMigrateTransport;
virCapabilitiesAddHostNUMACell;
[libvirt] [PATCHv3 6/6] test: Add support for thread and core information for the test driver
This patch adds demo processor topology information for the test driver.
---
src/test/test_driver.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6909fa4..ddc4110 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -558,7 +558,16 @@ static int testOpenDefault(virConnectPtr conn) {
privconn-cells[u].mem = (u + 1) * 2048 * 1024;
}
for (u = 0 ; u 16 ; u++) {
-privconn-cells[u % 2].cpus[(u / 2)].id = u;
+virBitmapPtr siblings = virBitmapNew(16);
+if (!siblings) {
+virReportOOMError();
+goto error;
+}
+ignore_value(virBitmapSetBit(siblings, u));
+privconn-cells[u / 8].cpus[(u % 8)].id = u;
+privconn-cells[u / 8].cpus[(u % 8)].socket_id = u / 8;
+privconn-cells[u / 8].cpus[(u % 8)].core_id = u % 8;
+privconn-cells[u / 8].cpus[(u % 8)].siblings = siblings;
}
if (!(privconn-caps = testBuildCapabilities(conn)))
--
1.8.1.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt -- add virtio-scsi disk with scsi-generic
Thank you~ I'd like to know when it is supported？ Is it on schedule？ 2013/1/24 Paolo Bonzini pbonz...@redhat.com: Il 24/01/2013 10:08, 韩诚 ha scritto: Hi, Paolo, All, I read virtio-scsi support proposal v2(http://permalink.gmane.org/gmane.comp.emulators.libvirt/50428). Try to add a virtio-scsi with scsi-generic like this: hostdev type='scsi' source adapter name='scsi_host0'/ address type='scsi' bus='0' target='0' unit='0'/ /source target address type='scsi' controller='0' bus='0' target='0' unit='2'/ /target /hostdev But It turn out to be wrong, showing: error: Failed to define domain from rhel63ga error: XML error: unknown host device source address type 'scsi_host' I'd like to know how shall to add a virtio-scsi disk with scsi-generic option. It's not yet supported by libvirt. Paolo -- Best regards, Cheng -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt -- add virtio-scsi disk with scsi-generic
2013/1/24 韩诚 hc0...@gmail.com: Hi, Paolo, All, I read virtio-scsi support proposal v2(http://permalink.gmane.org/gmane.comp.emulators.libvirt/50428). Try to add a virtio-scsi with scsi-generic like this: hostdev type='scsi' source adapter name='scsi_host0'/ address type='scsi' bus='0' target='0' unit='0'/ /source target address type='scsi' controller='0' bus='0' target='0' unit='2'/ /target /hostdev But It turn out to be wrong, showing: error: Failed to define domain from rhel63ga error: XML error: unknown host device source address type 'scsi_host' Sorry for mistake, scsi_host should be scsi I'd like to know how shall to add a virtio-scsi disk with scsi-generic option. Thanks~ -- Best regards, Cheng -- Best regards, Cheng -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt -- add virtio-scsi disk with scsi-generic
Hi, Paolo, All, I read virtio-scsi support proposal v2(http://permalink.gmane.org/gmane.comp.emulators.libvirt/50428). Try to add a virtio-scsi with scsi-generic like this: hostdev type='scsi' source adapter name='scsi_host0'/ address type='scsi' bus='0' target='0' unit='0'/ /source target address type='scsi' controller='0' bus='0' target='0' unit='2'/ /target /hostdev But It turn out to be wrong, showing: error: Failed to define domain from rhel63ga error: XML error: unknown host device source address type 'scsi_host' I'd like to know how shall to add a virtio-scsi disk with scsi-generic option. Thanks~ -- Best regards, Cheng -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 5/6] capabilities: Add additional data to the NUMA topology info
On Thu, Jan 24, 2013 at 10:57:33AM +0100, Peter Krempa wrote: diff --git a/src/nodeinfo.c b/src/nodeinfo.c index c29769c..5ce7d4f 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -79,9 +79,11 @@ freebsdNodeGetCPUCount(void) #ifdef __linux__ # define CPUINFO_PATH /proc/cpuinfo # define SYSFS_SYSTEM_PATH /sys/devices/system +# define SYSFS_CPU_PATH SYSFS_SYSTEM_PATH/cpu # define PROCSTAT_PATH /proc/stat # define MEMINFO_PATH /proc/meminfo # define SYSFS_MEMORY_SHARED_PATH /sys/kernel/mm/ksm +# define SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX 1024 s/LENGHT_MAX/LENGTH_MAX/ and where it is used later too. ACK with the typo fixed 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] [PATCHv2] qemu: Re-add driver unlock to qemuDomainSendKey
On 01/24/2013 10:42 AM, Michal Privoznik wrote: Why do we need qemu driver locked throughout whole API? I don't think we need so. If that's the case, we can simplify domain object lookup by switching to qemuDomObjFromDomain. That is: https://www.redhat.com/archives/libvir-list/2013-January/msg01760.html Michal You may be right but here I was mainly concerned in keeping the lock/unlock symmetry. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 0/6] Add additional data to NUMA topology info
On Thu, Jan 24, 2013 at 10:57:28AM +0100, Peter Krempa wrote: Another spin of the series. Difference to previous version: - remove redundant init of some structures - add graceful behavior if topology cannot be discovered - fix incorrect usage of virBitmapParse I'm working on adding more docs for the capabilities XML and the support for the xend driver. Peter Krempa (6): schema: Make the cpuset type reusable across schema files schemas: Add schemas for more CPU topology information in the caps XML conf: Split out NUMA topology formatting to simplify access to data capabilities: Switch CPU data in NUMA topology to a struct capabilities: Add additional data to the NUMA topology info test: Add support for thread and core information for the test driver ACK to all. For future reference, if you have a series and the early patches in the series have already been ACKed, there's no need to keep reposting the ACKd patches. Just merge the first ACKs ones in the series, and repost what is left over. 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 v3 0/2] selinux: Only create the selabel_handle once.
Version 3 of this patch. Because I now have to pass the 'mgr' pointer around, the patch is considerably more complicated than before. Patch 1/2 is required so that I can use virReportSystemError when I don't need to have any optional arguments, ie. the equivalent of: printf (foo\n); I have tested these two patches against the libguestfs test suite, and it works for me and removes the memory leak. Rich. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/2] selinux: Only create the selabel_handle once.
From: Richard W.M. Jones rjo...@redhat.com
According to Eric Paris this is slightly more efficient because it
only loads the regular expressions in libselinux once.
---
src/security/security_selinux.c | 129 ++--
1 file changed, 83 insertions(+), 46 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index a3ef728..d1f80b2 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -63,6 +63,9 @@ struct _virSecuritySELinuxData {
char *content_context;
virHashTablePtr mcs;
bool skipAllLabel;
+#if HAVE_SELINUX_LABEL_H
+struct selabel_handle *label_handle;
+#endif
};
struct _virSecuritySELinuxCallbackData {
@@ -367,12 +370,21 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr)
data-skipAllLabel = true;
+#if HAVE_SELINUX_LABEL_H
+data-label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
+if (!data-label_handle) {
+virReportSystemError(errno,
+ _(cannot open SELinux label_handle));
+return -1;
+}
+#endif
+
selinux_conf = virConfReadFile(selinux_lxc_contexts_path(), 0);
if (!selinux_conf) {
virReportSystemError(errno,
_(cannot open SELinux lxc contexts file '%s'),
selinux_lxc_contexts_path());
-return -1;
+goto error;
}
scon = virConfGetValue(selinux_conf, process);
@@ -418,6 +430,9 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr)
return 0;
error:
+#if HAVE_SELINUX_LABEL_H
+selabel_close(data-label_handle);
+#endif
virConfFree(selinux_conf);
VIR_FREE(data-domain_context);
VIR_FREE(data-file_context);
@@ -444,6 +459,15 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr)
data-skipAllLabel = false;
+#if HAVE_SELINUX_LABEL_H
+data-label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
+if (!data-label_handle) {
+virReportSystemError(errno,
+ _(cannot open SELinux label_handle));
+return -1;
+}
+#endif
+
if (virFileReadAll(selinux_virtual_domain_context_path(), MAX_CONTEXT,
(data-domain_context)) 0) {
virReportSystemError(errno,
_(cannot read SELinux virtual domain context
file '%s'),
@@ -499,6 +523,9 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr)
return 0;
error:
+#if HAVE_SELINUX_LABEL_H
+selabel_close(data-label_handle);
+#endif
VIR_FREE(data-domain_context);
VIR_FREE(data-alt_domain_context);
VIR_FREE(data-file_context);
@@ -763,6 +790,10 @@
virSecuritySELinuxSecurityDriverClose(virSecurityManagerPtr mgr)
if (!data)
return 0;
+#if HAVE_SELINUX_LABEL_H
+selabel_close(data-label_handle);
+#endif
+
virHashFree(data-mcs);
VIR_FREE(data-domain_context);
@@ -937,18 +968,13 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon)
/* Set fcon to the appropriate label for path and mode, or return -1. */
static int
-getContext(const char *newpath, mode_t mode, security_context_t *fcon)
+getContext(virSecurityManagerPtr mgr,
+ const char *newpath, mode_t mode, security_context_t *fcon)
{
#if HAVE_SELINUX_LABEL_H
-struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
-int ret;
-
-if (handle == NULL)
-return -1;
+virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
-ret = selabel_lookup_raw(handle, fcon, newpath, mode);
-selabel_close(handle);
-return ret;
+return selabel_lookup_raw(data-label_handle, fcon, newpath, mode);
#else
return matchpathcon(newpath, mode, fcon);
#endif
@@ -958,7 +984,8 @@ getContext(const char *newpath, mode_t mode,
security_context_t *fcon)
/* This method shouldn't raise errors, since they'll overwrite
* errors that the caller(s) are already dealing with */
static int
-virSecuritySELinuxRestoreSecurityFileLabel(const char *path)
+virSecuritySELinuxRestoreSecurityFileLabel(virSecurityManagerPtr mgr,
+ const char *path)
{
struct stat buf;
security_context_t fcon = NULL;
@@ -980,7 +1007,7 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char
*path)
goto err;
}
-if (getContext(newpath, buf.st_mode, fcon) 0) {
+if (getContext(mgr, newpath, buf.st_mode, fcon) 0) {
/* Any user created path likely does not have a default label,
* which makes this an expected non error
*/
@@ -997,7 +1024,7 @@ err:
}
static int
-virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
+virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainDiskDefPtr disk,
Re: [libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
On 01/24/2013 10:41 AM, Michal Privoznik wrote:
Currently, there is no reason to hold qemu driver locked
throughout whole API execution. Moreover, we can use the
new qemuDomObjFromDomain() internal API to lookup domain then.
---
src/qemu/qemu_driver.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6d4c1e9..100f10b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2407,19 +2407,12 @@ static int qemuDomainSendKey(virDomainPtr domain,
}
}
-qemuDriverLock(driver);
-vm = virDomainFindByUUID(driver-domains, domain-uuid);
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(domain-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(no domain with matching uuid '%s'), uuidstr);
+if (!(vm = qemuDomObjFromDomain(domain)))
goto cleanup;
-}
priv = vm-privateData;
-if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup;
if (!virDomainObjIsActive(vm)) {
@@ -2428,9 +2421,9 @@ static int qemuDomainSendKey(virDomainPtr domain,
goto endjob;
}
-qemuDomainObjEnterMonitorWithDriver(driver, vm);
+qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSendKey(priv-mon, holdtime, keycodes, nkeycodes);
-qemuDomainObjExitMonitorWithDriver(driver, vm);
+qemuDomainObjExitMonitor(driver, vm);
endjob:
if (qemuDomainObjEndJob(driver, vm) == 0)
@@ -2439,7 +2432,6 @@ endjob:
cleanup:
if (vm)
virObjectUnlock(vm);
-qemuDriverUnlock(driver);
return ret;
}
Formally this looks good and works on my system, so I would be fine with
it.
I am just wondering what the criteria are for holding a long-term lock
vs a short-time on. I.e. why would DomainSendKey be relaxed while
DomainInjectNMI keeps the driver lock?
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/2] Allow virReportSystemError to take no optional arguments.
From: Richard W.M. Jones rjo...@redhat.com --- src/util/virerror.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virerror.h b/src/util/virerror.h index 332a5eb..ac6d746 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -62,11 +62,11 @@ void virReportSystemErrorFull(int domcode, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(6, 7); -# define virReportSystemError(theerrno, fmt,...) \ +# define virReportSystemError(theerrno, ...) \ virReportSystemErrorFull(VIR_FROM_THIS, \ (theerrno), \ __FILE__, __FUNCTION__, __LINE__,\ - (fmt), __VA_ARGS__) + __VA_ARGS__) # define virReportInvalidNullArg(argname)\ virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, \ -- 1.8.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] selinux: Only create the selabel_handle once.
On Thu, Jan 24, 2013 at 10:10:56AM +, Richard W.M. Jones wrote: Version 3 of this patch. Because I now have to pass the 'mgr' pointer around, the patch is considerably more complicated than before. Patch 1/2 is required so that I can use virReportSystemError when I don't need to have any optional arguments, ie. the equivalent of: printf (foo\n); No, that's not allowed - everything must have a format string - even static messages - it should instead be: printf (%s, _(foo\n)); make syntax-check ought to have complained about this IIRC. 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/2] selinux: Only create the selabel_handle once.
On Thu, Jan 24, 2013 at 10:10:58AM +, Richard W.M. Jones wrote:
From: Richard W.M. Jones rjo...@redhat.com
According to Eric Paris this is slightly more efficient because it
only loads the regular expressions in libselinux once.
---
src/security/security_selinux.c | 129
++--
1 file changed, 83 insertions(+), 46 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index a3ef728..d1f80b2 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -63,6 +63,9 @@ struct _virSecuritySELinuxData {
char *content_context;
virHashTablePtr mcs;
bool skipAllLabel;
+#if HAVE_SELINUX_LABEL_H
+struct selabel_handle *label_handle;
+#endif
};
struct _virSecuritySELinuxCallbackData {
@@ -367,12 +370,21 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr
mgr)
data-skipAllLabel = true;
+#if HAVE_SELINUX_LABEL_H
+data-label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
+if (!data-label_handle) {
+virReportSystemError(errno,
+ _(cannot open SELinux label_handle));
This is missing %s,
+return -1;
+}
+#endif
+
selinux_conf = virConfReadFile(selinux_lxc_contexts_path(), 0);
if (!selinux_conf) {
virReportSystemError(errno,
_(cannot open SELinux lxc contexts file '%s'),
selinux_lxc_contexts_path());
-return -1;
+goto error;
}
scon = virConfGetValue(selinux_conf, process);
@@ -418,6 +430,9 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr)
return 0;
error:
+#if HAVE_SELINUX_LABEL_H
+selabel_close(data-label_handle);
+#endif
virConfFree(selinux_conf);
VIR_FREE(data-domain_context);
VIR_FREE(data-file_context);
@@ -444,6 +459,15 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr
mgr)
data-skipAllLabel = false;
+#if HAVE_SELINUX_LABEL_H
+data-label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
+if (!data-label_handle) {
+virReportSystemError(errno,
+ _(cannot open SELinux label_handle));
This is missing %s,
Rest of the patch looks fine though.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] selinux: Only create the selabel_handle once.
On Thu, Jan 24, 2013 at 10:17:16AM +, Daniel P. Berrange wrote: On Thu, Jan 24, 2013 at 10:10:56AM +, Richard W.M. Jones wrote: Version 3 of this patch. Because I now have to pass the 'mgr' pointer around, the patch is considerably more complicated than before. Patch 1/2 is required so that I can use virReportSystemError when I don't need to have any optional arguments, ie. the equivalent of: printf (foo\n); No, that's not allowed - everything must have a format string - even static messages - it should instead be: printf (%s, _(foo\n)); make syntax-check ought to have complained about this IIRC. I don't see why. Presumably the worry is that the translator will introduce a %-sequence into the string? That should be picked up by one of the msg* utilities. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
On 01/24/13 11:13, Viktor Mihajlovski wrote: On 01/24/2013 10:41 AM, Michal Privoznik wrote: Currently, there is no reason to hold qemu driver locked throughout whole API execution. Moreover, we can use the new qemuDomObjFromDomain() internal API to lookup domain then. --- src/qemu/qemu_driver.c | 16 1 file changed, 4 insertions(+), 12 deletions(-) Formally this looks good and works on my system, so I would be fine with it. I am just wondering what the criteria are for holding a long-term lock vs a short-time on. I.e. why would DomainSendKey be relaxed while DomainInjectNMI keeps the driver lock? That's due to historic reasons. The usual approach was to hold the driver lock all the time. We're trying to get rid of that. qemuDomainInjectNMI can be simplified too as it doesn't mess with the driver state. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
On 24.01.2013 11:13, Viktor Mihajlovski wrote:
On 01/24/2013 10:41 AM, Michal Privoznik wrote:
Currently, there is no reason to hold qemu driver locked
throughout whole API execution. Moreover, we can use the
new qemuDomObjFromDomain() internal API to lookup domain then.
---
src/qemu/qemu_driver.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6d4c1e9..100f10b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2407,19 +2407,12 @@ static int qemuDomainSendKey(virDomainPtr domain,
}
}
-qemuDriverLock(driver);
-vm = virDomainFindByUUID(driver-domains, domain-uuid);
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(domain-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(no domain with matching uuid '%s'), uuidstr);
+if (!(vm = qemuDomObjFromDomain(domain)))
goto cleanup;
-}
priv = vm-privateData;
-if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)
0)
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup;
if (!virDomainObjIsActive(vm)) {
@@ -2428,9 +2421,9 @@ static int qemuDomainSendKey(virDomainPtr domain,
goto endjob;
}
-qemuDomainObjEnterMonitorWithDriver(driver, vm);
+qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSendKey(priv-mon, holdtime, keycodes, nkeycodes);
-qemuDomainObjExitMonitorWithDriver(driver, vm);
+qemuDomainObjExitMonitor(driver, vm);
endjob:
if (qemuDomainObjEndJob(driver, vm) == 0)
@@ -2439,7 +2432,6 @@ endjob:
cleanup:
if (vm)
virObjectUnlock(vm);
-qemuDriverUnlock(driver);
return ret;
}
Formally this looks good and works on my system, so I would be fine with
it.
I am just wondering what the criteria are for holding a long-term lock
vs a short-time on. I.e. why would DomainSendKey be relaxed while
DomainInjectNMI keeps the driver lock?
Yes, that's another candidate.
Basically for now, everything that uses qemu driver other than:
- domain lookup
- begin/end job
- enter/exit monitor
SHOULD be using long term lock. This, however, will soon be past - I
believe I saw an e-mail from Dan saying, qemu driver locking should be
turned into R/W locks.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4] selinux: Only create the selabel_handle once.
From: Richard W.M. Jones rjo...@redhat.com
According to Eric Paris this is slightly more efficient because it
only loads the regular expressions in libselinux once.
---
src/security/security_selinux.c | 129 ++--
1 file changed, 83 insertions(+), 46 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index a3ef728..2affe69 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -63,6 +63,9 @@ struct _virSecuritySELinuxData {
char *content_context;
virHashTablePtr mcs;
bool skipAllLabel;
+#if HAVE_SELINUX_LABEL_H
+struct selabel_handle *label_handle;
+#endif
};
struct _virSecuritySELinuxCallbackData {
@@ -367,12 +370,21 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr)
data-skipAllLabel = true;
+#if HAVE_SELINUX_LABEL_H
+data-label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
+if (!data-label_handle) {
+virReportSystemError(errno, %s,
+ _(cannot open SELinux label_handle));
+return -1;
+}
+#endif
+
selinux_conf = virConfReadFile(selinux_lxc_contexts_path(), 0);
if (!selinux_conf) {
virReportSystemError(errno,
_(cannot open SELinux lxc contexts file '%s'),
selinux_lxc_contexts_path());
-return -1;
+goto error;
}
scon = virConfGetValue(selinux_conf, process);
@@ -418,6 +430,9 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr)
return 0;
error:
+#if HAVE_SELINUX_LABEL_H
+selabel_close(data-label_handle);
+#endif
virConfFree(selinux_conf);
VIR_FREE(data-domain_context);
VIR_FREE(data-file_context);
@@ -444,6 +459,15 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr)
data-skipAllLabel = false;
+#if HAVE_SELINUX_LABEL_H
+data-label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
+if (!data-label_handle) {
+virReportSystemError(errno, %s,
+ _(cannot open SELinux label_handle));
+return -1;
+}
+#endif
+
if (virFileReadAll(selinux_virtual_domain_context_path(), MAX_CONTEXT,
(data-domain_context)) 0) {
virReportSystemError(errno,
_(cannot read SELinux virtual domain context
file '%s'),
@@ -499,6 +523,9 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr)
return 0;
error:
+#if HAVE_SELINUX_LABEL_H
+selabel_close(data-label_handle);
+#endif
VIR_FREE(data-domain_context);
VIR_FREE(data-alt_domain_context);
VIR_FREE(data-file_context);
@@ -763,6 +790,10 @@
virSecuritySELinuxSecurityDriverClose(virSecurityManagerPtr mgr)
if (!data)
return 0;
+#if HAVE_SELINUX_LABEL_H
+selabel_close(data-label_handle);
+#endif
+
virHashFree(data-mcs);
VIR_FREE(data-domain_context);
@@ -937,18 +968,13 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon)
/* Set fcon to the appropriate label for path and mode, or return -1. */
static int
-getContext(const char *newpath, mode_t mode, security_context_t *fcon)
+getContext(virSecurityManagerPtr mgr,
+ const char *newpath, mode_t mode, security_context_t *fcon)
{
#if HAVE_SELINUX_LABEL_H
-struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0);
-int ret;
-
-if (handle == NULL)
-return -1;
+virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
-ret = selabel_lookup_raw(handle, fcon, newpath, mode);
-selabel_close(handle);
-return ret;
+return selabel_lookup_raw(data-label_handle, fcon, newpath, mode);
#else
return matchpathcon(newpath, mode, fcon);
#endif
@@ -958,7 +984,8 @@ getContext(const char *newpath, mode_t mode,
security_context_t *fcon)
/* This method shouldn't raise errors, since they'll overwrite
* errors that the caller(s) are already dealing with */
static int
-virSecuritySELinuxRestoreSecurityFileLabel(const char *path)
+virSecuritySELinuxRestoreSecurityFileLabel(virSecurityManagerPtr mgr,
+ const char *path)
{
struct stat buf;
security_context_t fcon = NULL;
@@ -980,7 +1007,7 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char
*path)
goto err;
}
-if (getContext(newpath, buf.st_mode, fcon) 0) {
+if (getContext(mgr, newpath, buf.st_mode, fcon) 0) {
/* Any user created path likely does not have a default label,
* which makes this an expected non error
*/
@@ -997,7 +1024,7 @@ err:
}
static int
-virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
+virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainDiskDefPtr disk,
Re: [libvirt] [PATCH v4] selinux: Only create the selabel_handle once.
On Thu, Jan 24, 2013 at 10:42:32AM +, Richard W.M. Jones wrote: From: Richard W.M. Jones rjo...@redhat.com According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 129 ++-- 1 file changed, 83 insertions(+), 46 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 0/6] Add additional data to NUMA topology info
On 01/24/13 11:05, Daniel P. Berrange wrote: On Thu, Jan 24, 2013 at 10:57:28AM +0100, Peter Krempa wrote: Another spin of the series. Difference to previous version: - remove redundant init of some structures - add graceful behavior if topology cannot be discovered - fix incorrect usage of virBitmapParse I'm working on adding more docs for the capabilities XML and the support for the xend driver. Peter Krempa (6): schema: Make the cpuset type reusable across schema files schemas: Add schemas for more CPU topology information in the caps XML conf: Split out NUMA topology formatting to simplify access to data capabilities: Switch CPU data in NUMA topology to a struct capabilities: Add additional data to the NUMA topology info test: Add support for thread and core information for the test driver ACK to all. For future reference, if you have a series and the early patches in the series have already been ACKed, there's no need to keep reposting the ACKd patches. Just merge the first ACKs ones in the series, and repost what is left over. Hm, yeah, those were pretty self-contained. I will keep that in mind for the next time. Thanks for the review. Peter Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] parallels: Resolve some resource leaks
On 23.01.2013 23:04, John Ferlan wrote:
Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within no_memory
label to be consistent
Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is
allocated locally. Also virCommandFree(cmd) as necessary.
---
src/parallels/parallels_driver.c | 47
1 file changed, 28 insertions(+), 19 deletions(-)
diff --git a/src/parallels/parallels_driver.c
b/src/parallels/parallels_driver.c
index 1b47246..9c88d71 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -271,17 +271,17 @@ parallelsAddVideoInfo(virDomainDefPtr def,
virJSONValuePtr value)
if (!(tmp = virJSONValueObjectGetString(value, size))) {
parallelsParseError();
-goto cleanup;
+goto error;
}
if (virStrToLong_ul(tmp, endptr, 10, mem) 0) {
parallelsParseError();
-goto cleanup;
+goto error;
}
if (!STREQ(endptr, Mb)) {
parallelsParseError();
-goto cleanup;
+goto error;
}
if (VIR_ALLOC(video) 0)
@@ -304,8 +304,9 @@ parallelsAddVideoInfo(virDomainDefPtr def,
virJSONValuePtr value)
no_memory:
virReportOOMError();
-cleanup:
+VIR_FREE(accel);
virDomainVideoDefFree(video);
While there is not much sense in virDomainVideoDefDree() here with
current code, I agree to use it rather than bare VIR_FREE() esp. when
the code might change and we will forget about it.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] security: Need to add virCommandFree()
On 23.01.2013 23:04, John Ferlan wrote: --- src/security/security_apparmor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 7331c91..6dcf6be 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -201,6 +201,7 @@ load_profile(virSecurityManagerPtr mgr, virCommandSetInputBuffer(cmd, xml); rc = virCommandRun(cmd, NULL); +virCommandFree(cmd); clean: VIR_FREE(xml); While technically, your patch is correct, I prefer having virCommandFree within cleanup label (yeah, there's just 'clean'). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Resolve parallels and virCommandPtr resource leaks
On 23.01.2013 23:04, John Ferlan wrote: This is v3 of the parallels_driver.c changes. The most recent review pointed out that virCommandPtr's need to be free'd. I found a few other instances where they weren't free'd. I also found a few instances where the status check from virCommandRun was not 0 and cleaned them to be more consistent. John Ferlan (5): parallels: Resolve some resource leaks security: Need to add virCommandFree() storage: Need to add virCommandFree() util: Need to add virCommandFree() parallels_utils: Check return status properly from virCommandRun() src/parallels/parallels_driver.c | 47 src/parallels/parallels_utils.c | 2 +- src/security/security_apparmor.c | 1 + src/storage/storage_backend_fs.c | 2 ++ src/util/virnetdevopenvswitch.c | 2 ++ 5 files changed, 34 insertions(+), 20 deletions(-) ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
On 01/24/2013 11:45 AM, Michal Privoznik wrote: Yes, that's another candidate. Basically for now, everything that uses qemu driver other than: - domain lookup - begin/end job - enter/exit monitor SHOULD be using long term lock. This, however, will soon be past - I believe I saw an e-mail from Dan saying, qemu driver locking should be turned into R/W locks. Michal Sounds reasonable. Knowing that this is *some* work, wouldn't it make sense to relax the locking for all eligible qemu_driver functions in one patch? This would then be a tangible feature of 1.0.2. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
On Thu, Jan 24, 2013 at 01:17:35PM +0100, Viktor Mihajlovski wrote: On 01/24/2013 11:45 AM, Michal Privoznik wrote: Yes, that's another candidate. Basically for now, everything that uses qemu driver other than: - domain lookup - begin/end job - enter/exit monitor SHOULD be using long term lock. This, however, will soon be past - I believe I saw an e-mail from Dan saying, qemu driver locking should be turned into R/W locks. Michal Sounds reasonable. Knowing that this is *some* work, wouldn't it make sense to relax the locking for all eligible qemu_driver functions in one patch? This would then be a tangible feature of 1.0.2. I'm killing the big qemu driver lock entirely in the release after this. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 1/2] Use g_strlcpy instead of strncpy
On Wed, Jan 23, 2013 at 03:41:56PM +0100, Martin Kletzander wrote: On 12/14/2012 05:03 PM, Christophe Fergeau wrote: This guarantees that the string will be nul-terminated. Coverity warned about this issue. --- libvirt-gobject/libvirt-gobject-connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 91cc535..0525323 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -1583,7 +1583,7 @@ GVirNodeInfo *gvir_connection_get_node_info(GVirConnection *conn, } ret = g_slice_new(GVirNodeInfo); -strncpy (ret-model, info.model, sizeof (ret-model)); +g_strlcpy (ret-model, info.model, sizeof (ret-model)); ret-memory = info.memory; ret-cpus = info.cpus; ret-mhz = info.mhz; OK, so nobody's having a look at this, so if I have the rights... ACK, Thanks for the review, I've pushed this now, Christophe pgpKh0CXpM115.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix build after commit 87b4c10c6cf02251dd8c29b5b895bebc6ec297f9
My gcc didn't whine about the uninitialized variable. This series tries to fix and then actually fixes the bug. Sorry for the inconvenience. I'm putting on my brown bag and upgrading my GCC. Series pushed as a build-breaker. Peter Krempa (2): xen: Initialize variable before using xen: Actually fix the uninitialized variable src/xen/xend_internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] xen: Initialize variable before using
Commit 87b4c10c6cf02251dd8c29b5b895bebc6ec297f9 added code that may call the virCapabilitiesClearHostNUMACellCPUTopology function with uninitialized second argument. Although the value wouldn't be used some compilers whine about that. --- Pushed as build-breaker. Thanks to John Ferlan for reporting. src/xen/xend_internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index d0e54a8..98ea6b4 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1115,7 +1115,7 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsHostNUMACellCPUPtr cpuInfo = NULL; int cell, cpu, nb_cpus; int n = 0; -int numCpus; +int numCpus = 0; nodeToCpu = sexpr_node(root, node/node_to_cpu); if (nodeToCpu == NULL) -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] xen: Actually fix the uninitialized variable
0eedb1d9bff672b24d6957dfaa0a8b42d9e851a7 fixed the wrong variable --- Pushed as a build-breaker fix. src/xen/xend_internal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 98ea6b4..b03b7bc 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1113,9 +1113,9 @@ sexpr_to_xend_topology(const struct sexpr *root, const char *nodeToCpu; const char *cur; virCapsHostNUMACellCPUPtr cpuInfo = NULL; -int cell, cpu, nb_cpus; +int cell, cpu, nb_cpus = 0; int n = 0; -int numCpus = 0; +int numCpus; nodeToCpu = sexpr_node(root, node/node_to_cpu); if (nodeToCpu == NULL) -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Make python objects inherit from 'object' base class
From: Daniel P. Berrange berra...@redhat.com As of python = 2.2, it is recommended that all objects inherit from the 'object' base class. We already require python = 2.3 for libvirt for thread macro support, so we should follow this best practice. See also http://stackoverflow.com/questions/4015417/python-class-inherits-object This is motivated the patch Rich just did for libguestfs https://www.redhat.com/archives/libguestfs/2013-January/msg00063.html Signed-off-by: Daniel P. Berrange berra...@redhat.com --- python/generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/generator.py b/python/generator.py index f853d77..a079fc5 100755 --- a/python/generator.py +++ b/python/generator.py @@ -1413,7 +1413,7 @@ def buildWrappers(module): classes.write(%s.__init__(self, _obj=_obj)\n\n % ( classes_ancestor[classname])) else: -classes.write(class %s:\n % (classname)) +classes.write(class %s(object):\n % (classname)) if classname in [ virDomain, virNetwork, virInterface, virStoragePool, virStorageVol, virNodeDevice, virSecret,virStream, virNWFilter ]: -- 1.8.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Two additional build fixes
Both pushed as trivial and build-breakers. Jiri Denemark (2): apparmor: Avoid freeing uninitialized pointer selinux: Properly indent preprocessor directives src/security/security_apparmor.c | 2 +- src/security/security_selinux.c | 8 2 files changed, 5 insertions(+), 5 deletions(-) -- 1.8.1.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] apparmor: Avoid freeing uninitialized pointer
---
src/security/security_apparmor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 2b9c337..c23ba87 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -150,61 +150,61 @@ profile_status_file(const char *str)
virReportOOMError();
goto failed;
}
if (strstr(content, tmp) != NULL)
rc = 0;
else
rc = 1;
failed:
VIR_FREE(tmp);
VIR_FREE(profile);
VIR_FREE(content);
return rc;
}
/*
* load (add) a profile. Will create one if necessary
*/
static int
load_profile(virSecurityManagerPtr mgr,
const char *profile,
virDomainDefPtr def,
const char *fn,
bool append)
{
int rc = -1;
bool create = true;
char *xml = NULL;
-virCommandPtr cmd;
+virCommandPtr cmd = NULL;
const char *probe = virSecurityManagerGetAllowDiskFormatProbing(mgr)
? 1 : 0;
xml = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE);
if (!xml)
goto cleanup;
if (profile_status_file(profile) = 0)
create = false;
cmd = virCommandNewArgList(VIRT_AA_HELPER, -p, probe,
create ? -c : -r,
-u, profile, NULL);
if (!create fn) {
if (append) {
virCommandAddArgList(cmd, -F, fn, NULL);
} else {
virCommandAddArgList(cmd, -f, fn, NULL);
}
}
virCommandSetInputBuffer(cmd, xml);
rc = virCommandRun(cmd, NULL);
cleanup:
VIR_FREE(xml);
virCommandFree(cmd);
return rc;
}
--
1.8.1.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make python objects inherit from 'object' base class
On Thu, Jan 24, 2013 at 01:17:32PM +, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com As of python = 2.2, it is recommended that all objects inherit from the 'object' base class. We already require python = 2.3 for libvirt for thread macro support, so we should follow this best practice. See also http://stackoverflow.com/questions/4015417/python-class-inherits-object This is motivated the patch Rich just did for libguestfs https://www.redhat.com/archives/libguestfs/2013-January/msg00063.html Signed-off-by: Daniel P. Berrange berra...@redhat.com --- python/generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/generator.py b/python/generator.py index f853d77..a079fc5 100755 --- a/python/generator.py +++ b/python/generator.py @@ -1413,7 +1413,7 @@ def buildWrappers(module): classes.write(%s.__init__(self, _obj=_obj)\n\n % ( classes_ancestor[classname])) else: -classes.write(class %s:\n % (classname)) +classes.write(class %s(object):\n % (classname)) if classname in [ virDomain, virNetwork, virInterface, virStoragePool, virStorageVol, virNodeDevice, virSecret,virStream, virNWFilter ]: ACK. I found a good discussion of what this is all about here: http://docs.python.org/release/2.2.3/whatsnew/sect-rellinks.html Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] This patch adds the label to lxc-enter-namespace
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
(Resend to the correct list)
lxc-enter-namespace allows a process from outside a container to start a
process inside a container. One problem with the current code is the process
running within the container would run with the label of the process that
created it.
For example if the admin process is running as unconfined_t and executes the
following command
# virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ
LABEL PID TTY TIME CMD
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient
staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps
Note the ps command is running as unconfined_t, After this patch,
virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ
LABEL PID TTY TIME CMD
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps
I also add a --nolabel command to virsh, which can go back to the original
behaviour.
virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ
LABEL PID TTY TIME CMD
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient
staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps
One problem I had when I originally did the patch is
lxcDomainGetSecurityLabel was returning the incorrect label, I needed the
label of the initpid within the container not its parent process, so I
changed this function to match OpenNamespaces function.
One last strangeness, about half the time I run this, virsh hangs and never
returns.
Seems like
if (conn-driver-domainGetSecurityLabel(domain,
seclabel) == 0) {
Gets hung up. I have attached the strace in out1.gz
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlEBO2EACgkQrlYvE4MpobMS5ACg3Ih4Iu0lD9BofF4iP0QXarAL
jpQAoLyWWNhnnFw2TRDJsXqvrTTVujyZ
=hUZ/
-END PGP SIGNATURE-
From e231257274c4716ea69a630b5613b91e04ebc813 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Tue, 22 Jan 2013 17:27:31 -0500
Subject: [PATCH 6/6] Set the label by default when entering a namespace, when
using SELinux.
Any lxc process that runs within the namespace should be running with the
context of the namespace.
I also added a --nolabel qualifier to virsh so you can run an unconfined_t
label in the namespace and not transition.
---
include/libvirt/libvirt-lxc.h | 4
src/libvirt-lxc.c | 44 ++-
tools/virsh-domain.c | 8 +++-
3 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt-lxc.h b/include/libvirt/libvirt-lxc.h
index f2c87fb..2fad400 100644
--- a/include/libvirt/libvirt-lxc.h
+++ b/include/libvirt/libvirt-lxc.h
@@ -32,6 +32,10 @@
extern C {
# endif
+typedef enum {
+SECURITY_LABEL = 1
+} NamespaceFlags;
+
int virDomainLxcOpenNamespace(virDomainPtr domain,
int **fdlist,
unsigned int flags);
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index f580c3c..2972721 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -29,6 +29,9 @@
#include virlog.h
#include virprocess.h
#include datatypes.h
+#ifdef WITH_SELINUX
+#include selinux/selinux.h
+#endif
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -103,6 +106,36 @@ error:
return -1;
}
+static int
+virDomainSetDefaultSecurityLabel(virDomainPtr domain)
+{
+int rc = 0;
+virSecurityLabelPtr seclabel;
+if (VIR_ALLOC(seclabel) 0)
+return -1;
+
+if (virDomainGetSecurityLabel(domain, seclabel))
+return -1;
+
+#ifdef WITH_SELINUX
+VIR_DEBUG(setexeccon %s, seclabel-label);
+rc = setexeccon(seclabel-label);
+if (rc)
+VIR_ERROR(_(Failed to set security context to %s), seclabel-label);
+#endif
+
+VIR_FREE(seclabel);
+
+return rc;
+}
+
+static void
+virDomainResetSecurityLabel(void)
+{
+#ifdef WITH_SELINUX
+(void) setexeccon(NULL);
+#endif
+}
/**
* virDomainLxcEnterNamespace:
@@ -135,7 +168,12 @@ virDomainLxcEnterNamespace(virDomainPtr domain,
{
int i;
-
Re: [libvirt] This patch adds the label to lxc-enter-namespace
On Thu, Jan 24, 2013 at 08:47:13AM -0500, Daniel J Walsh wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
(Resend to the correct list)
lxc-enter-namespace allows a process from outside a container to start a
process inside a container. One problem with the current code is the process
running within the container would run with the label of the process that
created it.
For example if the admin process is running as unconfined_t and executes the
following command
# virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ
LABEL PID TTY TIME CMD
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient
staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps
Note the ps command is running as unconfined_t, After this patch,
virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ
LABEL PID TTY TIME CMD
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps
I also add a --nolabel command to virsh, which can go back to the original
behaviour.
virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ
LABEL PID TTY TIME CMD
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal
system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient
staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps
One problem I had when I originally did the patch is
lxcDomainGetSecurityLabel was returning the incorrect label, I needed the
label of the initpid within the container not its parent process, so I
changed this function to match OpenNamespaces function.
One last strangeness, about half the time I run this, virsh hangs and never
returns.
Seems like
if (conn-driver-domainGetSecurityLabel(domain,
seclabel) == 0) {
Gets hung up. I have attached the strace in out1.gz
virDomainLxcEnterNamespace is run in the child process context - it is
forbidden to use the RPC connection once you've forked, becuase it
belongs to the parent process. The reason for the random hang is that
sometimes your child process reads the incoming I/O, sometimes the
parent process reads the incoming I/O.
This has to be implemented entirely in virsh, rather than in the
virDomainLxcEnterNamespace API. virsh should call virDomainGetSecurityLabel
before forking, and then call setexeccon inside the child process with
the data it obtained. That way no libvirt RPC calls take place in the
child.
@@ -7719,13 +7721,17 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd
*cmd)
*/
if (virFork(pid) 0)
goto cleanup;
+
+if (!vshCommandOptBool(cmd, nolabel))
+flags |= SECURITY_LABEL;
+
if (pid == 0) {
...at this point you're in the child process...
if (virDomainLxcEnterNamespace(dom,
nfdlist,
fdlist,
NULL,
NULL,
- 0) 0)
+ flags) 0)
_exit(255);
/* Fork a second time because entering the
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt -- add virtio-scsi disk with scsi-generic
On 2013年01月24日 17:46, 韩诚 wrote: Thank you~ I'd like to know when it is supported？ Is it on schedule？ Right, it's not supported yet, it's on my TODO list (yes, on schedule), but it will be after the patches for Migration with NPIV. Even might be after (or before) integration of ivshmem server into libvirt. So you may have to wait for a while. 2013/1/24 Paolo Bonzinipbonz...@redhat.com: Il 24/01/2013 10:08, 韩诚 ha scritto: Hi, Paolo, All, I read virtio-scsi support proposal v2(http://permalink.gmane.org/gmane.comp.emulators.libvirt/50428). Try to add a virtio-scsi with scsi-generic like this: hostdev type='scsi' source adapter name='scsi_host0'/ address type='scsi' bus='0' target='0' unit='0'/ /source target address type='scsi' controller='0' bus='0' target='0' unit='2'/ /target /hostdev But It turn out to be wrong, showing: error: Failed to define domain from rhel63ga error: XML error: unknown host device source address type 'scsi_host' I'd like to know how shall to add a virtio-scsi disk with scsi-generic option. It's not yet supported by libvirt. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-java] Add various block, snapshot and migrate methods
At Wed, 23 Jan 2013 16:19:43 +0100, Wido den Hollander wrote: Hello Claudio, On 01/23/2013 04:13 PM, Claudio Bley wrote: Hello Wido, At Sun, 13 Jan 2013 19:09:22 +0100, Wido den Hollander wrote: Hi, I've sent a series of patches like these about two weeks ago and got some great feedback from Claudio on those! The feedback from Claudio has been used for writing this series of patches. Backwards compatibility has been preserved by still using virDomainMigrate and virtDomainMigrateToUri for the exisiting methods, just to be sure. I have reviewed your changeset and basically it's OK with a few nits fixed. If you or somebody else doesn't have any objections I'd push the whole series. Thank you! I just saw your comments coming in. Since I don't know what the convention is for libvirt-java I followed most of the existing code with the implementation. Your feedback sounds good, so no objections from my side. OK, pushed now. Thanks for your submission! Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:http://www.av-test.org 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
[libvirt] not able to stop vmware player node through libvirt API
Hi, I have started one node through libvirt API using its start method. I am trying to stop that node using the *destroy *method provided by the API but I am not able to do so. Instead, I am getting one message saying: *libvir: error: internal error Child process (vmrun -T player stop /root/test_folder/myImage.vmx soft) unexpected exit status 255* Can anybody tell me why I am not able to stop the node. Thanks in advance. :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [TCK] [PATCH] Add --physdev-is-bridged to test cases
Follow recent changes in libvirt and add --physdev-is-bridged to test cases where needed. --- scripts/nwfilter/nwfilterxml2fwallout/ah-ipv6-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/ah-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/all-ipv6-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/all-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/comment-test.fwall |8 scripts/nwfilter/nwfilterxml2fwallout/conntrack-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/esp-ipv6-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/esp-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/hex-data-test.fwall|8 scripts/nwfilter/nwfilterxml2fwallout/icmp-direction-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/icmp-direction2-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/icmp-direction3-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/icmp-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/icmpv6-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/igmp-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/ipset-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/ipt-no-macspoof-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/iter-test1.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/iter-test2.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/iter-test3.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/sctp-ipv6-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/sctp-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/target-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/target-test2.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/tcp-ipv6-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/tcp-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/testvm.fwall.dat |4 ++-- scripts/nwfilter/nwfilterxml2fwallout/udp-ipv6-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/udp-test.fwall |2 +- scripts/nwfilter/nwfilterxml2fwallout/udplite-ipv6-test.fwall|2 +- scripts/nwfilter/nwfilterxml2fwallout/udplite-test.fwall |2 +- 31 files changed, 38 insertions(+), 38 deletions(-) Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ah-ipv6-test.fwall === --- libvirt-tck.orig/scripts/nwfilter/nwfilterxml2fwallout/ah-ipv6-test.fwall +++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ah-ipv6-test.fwall @@ -25,4 +25,4 @@ FI-vnet0 all ::/0 ::/0 [goto] PHYSDEV ma #ip6tables -L libvirt-in-post -n | grep vnet0 ACCEPT all ::/0 ::/0PHYSDEV match --physdev-in vnet0 #ip6tables -L libvirt-out -n | grep vnet0 | tr -s -FO-vnet0 all ::/0 ::/0 [goto] PHYSDEV match --physdev-out vnet0 +FO-vnet0 all ::/0 ::/0 [goto] PHYSDEV match --physdev-out vnet0 --physdev-is-bridged Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ah-test.fwall === --- libvirt-tck.orig/scripts/nwfilter/nwfilterxml2fwallout/ah-test.fwall +++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/ah-test.fwall @@ -23,4 +23,4 @@ FI-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [got #iptables -L libvirt-in-post -n | grep vnet0 ACCEPT all -- 0.0.0.0/00.0.0.0/0 PHYSDEV match --physdev-in vnet0 #iptables -L libvirt-out -n | grep vnet0 | tr -s -FO-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-out vnet0 +FO-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-out vnet0 --physdev-is-bridged Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/all-ipv6-test.fwall === --- libvirt-tck.orig/scripts/nwfilter/nwfilterxml2fwallout/all-ipv6-test.fwall +++ libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/all-ipv6-test.fwall @@ -25,7 +25,7 @@ FI-vnet0 all ::/0 ::/0 [goto] PHYSDEV ma #ip6tables -L libvirt-in-post -n | grep vnet0 ACCEPT all ::/0 ::/0PHYSDEV match --physdev-in vnet0 #ip6tables -L libvirt-out -n | grep vnet0 | tr -s -FO-vnet0 all ::/0 ::/0 [goto] PHYSDEV match --physdev-out vnet0 +FO-vnet0 all ::/0 ::/0 [goto] PHYSDEV match --physdev-out vnet0 --physdev-is-bridged #ip6tables -L FORWARD --line-number | grep libvirt 1libvirt-in all anywhere anywhere 2libvirt-out all anywhere anywhere Index: libvirt-tck/scripts/nwfilter/nwfilterxml2fwallout/all-test.fwall === ---
[libvirt] Entering freeze for libvirt-1.0.2
As scheduled and to gte a release by the end of the month next week, I have just tagged a release candidate 1 version in git and pushed associated tarballs and rpms to the usual place: ftp://libvirt.org/libvirt/ please give it a try and let's ocuse on fixing bugs untl the freeze end. I will probably make a release candidate 2 on Monday, and if everything goes well rol out the 1.0.2 release on Wednesday or Thursday. thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] Introduce new XMLs to specify disk source using storage pool/vol
On 01/23/2013 10:24 AM, Osier Yang wrote: On 2013年01月23日 22:58, Daniel P. Berrange wrote: On Wed, Jan 23, 2013 at 10:55:25PM +0800, Osier Yang wrote: On 2013年01月23日 22:18, Daniel P. Berrange wrote: On Wed, Jan 23, 2013 at 07:04:35PM +0800, Osier Yang wrote: With this patch, one can specify the disk source using storage pool/volume like: disk type='file' device='disk' driver name='qemu' type='raw' cache='none'/ source type='pool' volume key='/var/lib/libvirt/images/foo.img'/ seclabel relabel='no'/ /source target dev='vdb' bus='virtio'/ /disk disk type='file' device='disk' driver name='qemu' type='raw' cache='none'/ source type='pool' volume path='/var/lib/libvirt/images/foo.img'/ seclabel relabel='no'/ /source target dev='vdb' bus='virtio'/ /disk disk type='file' device='disk' driver name='qemu' type='raw' cache='none'/ source type='pool' pool uuid|name=$var/ volume name='foo.img'/ seclabel relabel='no'/ /source target dev='vdb' bus='virtio'/ /disk If you're going to introduce new schema forsource, then you must introduce a new disk type value.ie a disk type='file' must always use thesource file='...'/ XML syntax, otherwise you cause backwards compatibility problems for applications Oh, yes. I need a v2. What you need here is adisk type='volume'/ for your new schema. But before I make up the v2, do you see other design problem on the set? Thanks. I'm wondering if it is really requires to allow so many different options for specifyin the pool volume. Forinterface type='network' we were fine simply using the 'name' and ignoring UUID. I cna't help thinking that for storage we can similarly just use the pool name and volume name This was my hesitating too when on the half road. But to post the RFC earlier, and considering it's at least not a bad thing, as we provide all the interfaces, so I went on with it. I think it makes no big difference if we simply use pool name and volume name, but what I'm not sure is if the users will want the uuid for pool, and path/key for volume (using path/key is convenient as the pool is not even neccessary). (Keep in mind this is coming from a non-storage guy, so there may be some flaws in my logic :-) Too many ways of describing the same thing is bad, as it leads to confusion. Also, since the point of making this abstraction is to isolate the host-specific details of the device from the domain's configuration, I think allowing the file path to be specified is counter-productive (since it could be different from host to host, depending on where an NFS share is mounted, for example). Of course this is a bit of a different situation than network devices, since the pool/volume must end up pointing to the same bits from all hosts (either the exact same bits via a different access path, or a new copy of the bits migrated over to a different type of storage), but in the end it should be possible for the disk image to be in a local directory on one host, accessed by NFS on another, and maybe even via iscsi or lvm on another - those details should all be in the pool/volume definitions on each host, and the guest config should just say this disk's image is in pool x, volume y. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK] [PATCH] Add --physdev-is-bridged to test cases
On 01/24/2013 11:34 AM, Stefan Berger wrote: Follow recent changes in libvirt and add --physdev-is-bridged to test cases where needed. ACK. (Does this mean that new libvirt-tck will fail when run against an older libvirt, though?) By the way, when the patch went into libvirt, the person who posted it mentioned that when restarting libvirtd after the first upgrade with that patch, the existing rules wouldn't get removed because they wouldn't be an exact match to what libvirt was trying to remove: On 01/18/2013 02:44 AM, Reinier Schoof wrote: On a side note, please be aware that when upgrading to a libvirt version with this patch included, libvirt will not be able to remove the earlier ip(6)tables rules without the '--physdev-is-bridged' addition. When restarting libvirt, it will look for rules that match with '--physdev-is-bridged' and since that wasn't there before, you'll end up with a duplicate/malfunctioning ruleset. You'll have to remove these rules/chains manually. Is this actually a problem? I had thought that nwfilter always removed entire chains instead of individual rules. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
On 01/24/2013 02:41 AM, Michal Privoznik wrote: Currently, there is no reason to hold qemu driver locked throughout whole API execution. Moreover, we can use the new qemuDomObjFromDomain() internal API to lookup domain then. --- src/qemu/qemu_driver.c | 16 1 file changed, 4 insertions(+), 12 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] selinux: Only create the selabel_handle once.
On 01/24/2013 03:24 AM, Richard W.M. Jones wrote: On Thu, Jan 24, 2013 at 10:17:16AM +, Daniel P. Berrange wrote: On Thu, Jan 24, 2013 at 10:10:56AM +, Richard W.M. Jones wrote: Version 3 of this patch. Because I now have to pass the 'mgr' pointer around, the patch is considerably more complicated than before. Patch 1/2 is required so that I can use virReportSystemError when I don't need to have any optional arguments, ie. the equivalent of: printf (foo\n); No, that's not allowed - everything must have a format string - even static messages - it should instead be: printf (%s, _(foo\n)); make syntax-check ought to have complained about this IIRC. I don't see why. Presumably the worry is that the translator will introduce a %-sequence into the string? That should be picked up by one of the msg* utilities. Whether or not it is picked up by the msg* utilities, there is also the issue of whether xgettext will mark the string as being printf-format when extracting the string for translation, as well as the issue that there are existing compilers (at least the older gcc on FreeBSD) that loudly complain (and thus break a -Werror build) if given a function marked __attribute__((__printf__(...))) but with no % in the format string. So just because the standard allows you to use printf with a format string with no % doesn't mean that we like that style. -- 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 v2] python: Fix bindings for virDomainSnapshotGet{Domain, Connect}
On 01/23/2013 04:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect()
wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed
to be ever implemented. The class should contain proper domain() and
connect() accessors that fetch python objects stored internally within
the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and
reimplements getDomain() and getConnect() methods as aliases to domain()
and connect() for backward compatibility.
---
python/generator.py | 4 +++-
python/libvirt-override-virDomainSnapshot.py | 8
src/libvirt.c| 10 --
3 files changed, 19 insertions(+), 3 deletions(-)
* virDomainSnapshotGetDomain:
* @snapshot: a snapshot object
*
- * Get the domain that a snapshot was created for
+ * Get the domain that a snapshot was created for.
+ *
Missing the following (based on copy-and-paste from virDomainGetConnect):
* Provides the domain pointer associated with a snapshot. The
* reference counter on the domain is not increased by this
* call.
+ * WARNING: When writing libvirt bindings in other languages, do not use this
+ * function. Instead, store the domain and the snapshot object together.
*
* Returns the domain or NULL.
*/
@@ -17874,7 +17877,10 @@ virDomainSnapshotGetDomain(virDomainSnapshotPtr
snapshot)
* virDomainSnapshotGetConnect:
* @snapshot: a snapshot object
*
- * Get the connection that owns the domain that a snapshot was created for
+ * Get the connection that owns the domain that a snapshot was created for.
+ *
Likewise:
* Provides the connection pointer associated with a snapshot. The
* reference counter on the connection is not increased by this
* call.
+ * WARNING: When writing libvirt bindings in other languages, do not use this
+ * function. Instead, store the connection and the snapshot object together.
*
* Returns the connection or NULL.
*/
ACK with those additions; no need to see a v3.
--
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] Fix performance reliabilty of QMP probing
From: Daniel P. Berrange berra...@redhat.com
This previous commit
commit 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9
Author: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
Date: Mon Nov 26 15:17:13 2012 +0100
qemu: Fix QMP Capabability Probing Failure
which attempted to make sure the QEMU process used for probing
ran as the right user id, caused serious performance regression
and unreliability in probing. The -daemonize switch in QEMU
guarantees that the monitor socket is present before the parent
process exits. This means libvirtd is guaranteed to be able to
connect immediately. By switching from -daemonize to the
virCommandDaemonize API libvirtd was no longer synchronized with
QEMU's startup process. The result was that the QEMU monitor
failed to open and went into its 200ms sleep loop. This happened
for all 25 binaries resulting in 5 seconds worth of sleeping
at libvirtd startup. In addition sometimes when libvirt connected,
QEMU would be partially initialized and crash causing total
failure to probe that binary.
This commit reverts the previous change, ensuring we do use the
-daemonize flag to QEMU. Startup delay is cut from 7 seconds
to 2 seconds on my machine, which is on a par with what it was
prior to the capabilities rewrite.
To deal with the fact that QEMU needs to be able to create the
pidfile, we switch pidfile location fron runDir to libDir, which
QEMU is guaranteed to be able to write to.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_capabilities.c | 50 ++--
src/qemu/qemu_capabilities.h | 3 +--
2 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 95fa3da..703179d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -38,6 +38,7 @@
#include virbitmap.h
#include virnodesuspend.h
#include qemu_monitor.h
+#include virtime.h
#include sys/stat.h
#include unistd.h
@@ -916,11 +917,19 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache)
* so just probe for them all - we gracefully fail
* if a qemu-system-$ARCH binary can't be found
*/
-for (i = 0 ; i VIR_ARCH_LAST ; i++)
+unsigned long long a, b;
+for (i = 0 ; i VIR_ARCH_LAST ; i++) {
+unsigned long long start, end;
+if (virTimeMillisNow(start) 0)
+goto error;
if (qemuCapsInitGuest(caps, cache,
virArchFromHost(),
i) 0)
goto error;
+if (virTimeMillisNow(end) 0)
+goto error;
+VIR_DEBUG(Probed %s in %llums, virArchToString(i), end-start);
+}
/* QEMU Requires an emulator in the XML */
virCapabilitiesSetEmulatorRequired(caps);
@@ -2291,7 +2300,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
static int
qemuCapsInitQMP(qemuCapsPtr caps,
const char *libDir,
-const char *runDir,
uid_t runUid,
gid_t runGid)
{
@@ -2324,8 +2332,11 @@ qemuCapsInitQMP(qemuCapsPtr caps,
/* .pidfile suffix is used rather than .pid to avoid a possible clash
* with a qemu domain called capabilities
+ * Normally we'd use runDir for pid files, but because we're using
+ * -daemonize we need QEMU to be allowed to create them, rather
+ * than libvirtd. So we're using libDir which QEMU can write to
*/
-if (virAsprintf(pidfile, %s/%s, runDir, capabilities.pidfile) 0) {
+if (virAsprintf(pidfile, %s/%s, libDir, capabilities.pidfile) 0) {
virReportOOMError();
goto cleanup;
}
@@ -2337,6 +2348,13 @@ qemuCapsInitQMP(qemuCapsPtr caps,
VIR_DEBUG(Try to get caps via QMP caps=%p, caps);
+/*
+ * We explicitly need to use -daemonize here, rather than
+ * virCommandDaemonize, because we need to synchronize
+ * with QEMU creating its monitor socket API. Using
+ * daemonize guarnatees control won't return to libvirt
+ * until the socket is present.
+ */
cmd = virCommandNewArgList(caps-binary,
-S,
-no-user-config,
@@ -2344,14 +2362,14 @@ qemuCapsInitQMP(qemuCapsPtr caps,
-nographic,
-M, none,
-qmp, monarg,
+ -pidfile, pidfile,
+ -daemonize,
NULL);
virCommandAddEnvPassCommon(cmd);
virCommandClearCaps(cmd);
hookData.runUid = runUid;
hookData.runGid = runGid;
virCommandSetPreExecHook(cmd, qemuCapsHook, hookData);
-virCommandSetPidFile(cmd, pidfile);
-virCommandDaemonize(cmd);
if (virCommandRun(cmd, status) 0)
goto cleanup;
@@ -2472,7 +2490,6 @@ cleanup:
qemuCapsPtr qemuCapsNewForBinary(const char *binary,
const char *libDir,
-
[libvirt] [PATCH] Move QEMU capabilities initialization later in QEMU startup
From: Daniel P. Berrange berra...@redhat.com Currently QEMU capabilities are initialized before the QEMU driver sets ownership on its various directories. The upshot is that if you change the user/group in the qemu.conf file, libvirtd will fail to probe QEMU the first time it is run after the config change. Moving QEMU capabilities initialization to after the chown() calls fixes this Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_driver.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1f43d54..9ff920e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -825,13 +825,6 @@ qemuStartup(bool privileged, if (qemuSecurityInit(qemu_driver) 0) goto error; -qemu_driver-capsCache = qemuCapsCacheNew(qemu_driver-libDir, - qemu_driver-stateDir, - qemu_driver-user, - qemu_driver-group); -if (!qemu_driver-capsCache) -goto error; - if ((qemu_driver-activePciHostdevs = pciDeviceListNew()) == NULL) goto error; @@ -871,6 +864,12 @@ qemuStartup(bool privileged, } } +qemu_driver-capsCache = qemuCapsCacheNew(qemu_driver-libDir, + qemu_driver-user, + qemu_driver-group); +if (!qemu_driver-capsCache) +goto error; + if ((qemu_driver-caps = qemuCreateCapabilities(qemu_driver)) == NULL) goto error; -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] node_memory: Add '\n' to help message
Linefeed is missed in the help of node-memory-tune.
This patch just adds '\n' to get a correct help message.
Signed-off-by: Satoru Moriya satoru.mor...@hds.com
---
tools/virsh-host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c index d05e435..b83c893
100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -764,7 +764,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
static const vshCmdInfo info_node_memory_tune[] = {
{help, N_(Get or set node memory parameters)},
-{desc, N_(Get or set node memory parameters
+{desc, N_(Get or set node memory parameters\n
To get the memory parameters, use following command: \n\n
virsh # node-memory-tune)},
{NULL, NULL}
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] libxl: Find domain object in event handler
Jim Fehlig wrote:
Since libxl provides the domain ID in the event handler callback,
find the domain object based on the ID. This approach prevents
processing the callback on a domain that has already been reaped.
---
src/libxl/libxl_driver.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7484b83..e28b641 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -656,22 +656,22 @@ libxlVmReap(libxlDriverPrivatePtr driver,
* Handle previously registered event notification from libxenlight
*/
static void
-libxlEventHandler(void *data, const libxl_event *event)
+libxlEventHandler(void *data ATTRIBUTE_UNUSED, const libxl_event *event)
{
libxlDriverPrivatePtr driver = libxl_driver;
-virDomainObjPtr vm = data;
+virDomainObjPtr vm = NULL;
virDomainEventPtr dom_event = NULL;
libxlDriverLock(driver);
On xen-unstable, I noticed an occasional deadlock here when libxl
invokes the event handler with a SUSPEND shutdown reason. The driver
lock is already held by the caller of libxl_domain_suspend(). Inspired
by the xl implementation, I've updated this patch to ignore the SUSPEND
shutdown reason before acquiring the driver lock.
Regards,
Jim
From b9b3cd0259168ccf10f525d1b459bfe790162be3 Mon Sep 17 00:00:00 2001
From: Jim Fehlig jfeh...@suse.com
Date: Mon, 21 Jan 2013 10:36:03 -0700
Subject: [PATCH 6/6] libxl: Domain event handler improvements
Since libxl provides the domain ID in the event handler callback,
find the domain object based on the ID. This approach prevents
processing the callback on a domain that has already been reaped.
Also, similar to the xl implementation, ignore the SUSPEND shutdown
reason. By calling libxl_domain_suspend(), we know a shutdown
event with SUSPEND reason will be generated, but it can be safely
ignored since any subsequent cleanup will be done by the callers.
---
src/libxl/libxl_driver.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7484b83..39875aa 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -656,26 +656,32 @@ libxlVmReap(libxlDriverPrivatePtr driver,
* Handle previously registered event notification from libxenlight
*/
static void
-libxlEventHandler(void *data, const libxl_event *event)
+libxlEventHandler(void *data ATTRIBUTE_UNUSED, const libxl_event *event)
{
libxlDriverPrivatePtr driver = libxl_driver;
-virDomainObjPtr vm = data;
+virDomainObjPtr vm = NULL;
virDomainEventPtr dom_event = NULL;
-
-libxlDriverLock(driver);
-virObjectLock(vm);
-libxlDriverUnlock(driver);
+libxl_shutdown_reason xl_reason = event-u.domain_shutdown.shutdown_reason;
if (event-type == LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) {
virDomainShutoffReason reason;
-if (event-domid != vm-def-id)
+/* Similar to the xl implementation, ignore SUSPEND. Any actions needed
+ after calling libxl_domain_suspend() are handled by it's callers. */
+if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND)
+goto cleanup;
+
+libxlDriverLock(driver);
+vm = virDomainFindByID(driver-domains, event-domid);
+libxlDriverUnlock(driver);
+
+if (!vm)
goto cleanup;
-switch (event-u.domain_shutdown.shutdown_reason) {
+switch (xl_reason) {
case LIBXL_SHUTDOWN_REASON_POWEROFF:
case LIBXL_SHUTDOWN_REASON_CRASH:
-if (event-u.domain_shutdown.shutdown_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
+if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
dom_event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
@@ -694,7 +700,7 @@ libxlEventHandler(void *data, const libxl_event *event)
libxlVmStart(driver, vm, 0, -1);
break;
default:
-VIR_INFO(Unhandled shutdown_reason %d, event-u.domain_shutdown.shutdown_reason);
+VIR_INFO(Unhandled shutdown_reason %d, xl_reason);
break;
}
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] libxl: Fix handling of timeouts
Jim Fehlig wrote:
xen-unstable commit makes changes wrt modifying and deregistering
timeouts.
First, timeout modify callbacks will only be invoked with an
abs_t of {0,0}, i.e. make the timeout fire immediately. Prior to this
commit, timeout modify callbacks were never invoked.
Second, timeout deregister hooks will no longer be called.
This patch makes changes in the libvirt libxl driver that should be
compatible before and after commit .
While at it, fix a potential overflow in the timeout register callback.
---
'commit ' will be replaced with a proper commit id once committed
to xen-unstable.
libxl patch as been committed to xen-unstable now, changeset 26469.
I've updated this patch accordingly.
Regards,
Jim
From 451c84284fa1b4f311f2ceb052c8e9f25ffa0cf0 Mon Sep 17 00:00:00 2001
From: Jim Fehlig jfeh...@suse.com
Date: Mon, 21 Jan 2013 09:59:28 -0700
Subject: [PATCH 1/6] libxl: Fix handling of timeouts
xen-unstable changeset 26469 makes changes wrt modifying and deregistering
timeouts.
First, timeout modify callbacks will only be invoked with an
abs_t of {0,0}, i.e. make the timeout fire immediately. Prior to this
commit, timeout modify callbacks were never invoked.
Second, timeout deregister hooks will no longer be called.
This patch makes changes in the libvirt libxl driver that should be
compatible before and after changeset 26469.
While at it, fix a potential overflow in the timeout register callback.
---
src/libxl/libxl_driver.c | 39 ++-
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a8c4cae..77026fc 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -186,7 +186,13 @@ libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
{
struct libxlOSEventHookTimerInfo *info = timer_info;
+/* libxl expects the event to be deregistered when calling
+ libxl_osevent_occurred_timeout, but we dont want the event info
+ destroyed. Disable the timeout and only remove it after returning
+ from libxl. */
+virEventUpdateTimeout(info-id, -1);
libxl_osevent_occurred_timeout(info-priv-ctx, info-xl_priv);
+virEventRemoveTimeout(info-id);
}
static void
@@ -202,6 +208,8 @@ libxlTimeoutRegisterEventHook(void *priv,
void *xl_priv)
{
struct timeval now;
+struct timeval res;
+static struct timeval zero;
struct libxlOSEventHookTimerInfo *timer_info;
int timeout, timer_id;
@@ -211,8 +219,15 @@ libxlTimeoutRegisterEventHook(void *priv,
}
gettimeofday(now, NULL);
-timeout = (abs_t.tv_usec - now.tv_usec) / 1000;
-timeout += (abs_t.tv_sec - now.tv_sec) * 1000;
+timersub(abs_t, now, res);
+/* Ensure timeout is not overflowed */
+if (timercmp(res, zero, )) {
+timeout = 0;
+} else if (res.tv_sec INT_MAX / 1000) {
+timeout = INT_MAX;
+} else {
+timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
+}
timer_id = virEventAddTimeout(timeout, libxlTimerCallback,
timer_info, libxlTimerInfoFree);
if (timer_id 0) {
@@ -227,19 +242,25 @@ libxlTimeoutRegisterEventHook(void *priv,
return 0;
}
+/*
+ * Note: There are two changes wrt timeouts starting with xen-unstable
+ * changeset 26469:
+ *
+ * 1. Timeout modify callbacks will only be invoked with an abs_t of {0,0},
+ * i.e. make the timeout fire immediately. Prior to this commit, timeout
+ * modify callbacks were never invoked.
+ *
+ * 2. Timeout deregister hooks will no longer be called.
+ */
static int
libxlTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
void **hndp,
-struct timeval abs_t)
+struct timeval abs_t ATTRIBUTE_UNUSED)
{
-struct timeval now;
-int timeout;
struct libxlOSEventHookTimerInfo *timer_info = *hndp;
-gettimeofday(now, NULL);
-timeout = (abs_t.tv_usec - now.tv_usec) / 1000;
-timeout += (abs_t.tv_sec - now.tv_sec) * 1000;
-virEventUpdateTimeout(timer_info-id, timeout);
+/* Make the timeout fire */
+virEventUpdateTimeout(timer_info-id, 0);
return 0;
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix performance reliabilty of QMP probing
On 01/24/2013 11:34 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
This commit reverts the previous change, ensuring we do use the
-daemonize flag to QEMU. Startup delay is cut from 7 seconds
to 2 seconds on my machine, which is on a par with what it was
prior to the capabilities rewrite.
To deal with the fact that QEMU needs to be able to create the
pidfile, we switch pidfile location fron runDir to libDir, which
QEMU is guaranteed to be able to write to.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_capabilities.c | 50
++--
src/qemu/qemu_capabilities.h | 3 +--
2 files changed, 35 insertions(+), 18 deletions(-)
@@ -916,11 +917,19 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache)
* so just probe for them all - we gracefully fail
* if a qemu-system-$ARCH binary can't be found
*/
-for (i = 0 ; i VIR_ARCH_LAST ; i++)
+unsigned long long a, b;
What are 'a' and 'b' for?
+for (i = 0 ; i VIR_ARCH_LAST ; i++) {
+unsigned long long start, end;
+if (virTimeMillisNow(start) 0)
+goto error;
if (qemuCapsInitGuest(caps, cache,
virArchFromHost(),
i) 0)
goto error;
+if (virTimeMillisNow(end) 0)
+goto error;
+VIR_DEBUG(Probed %s in %llums, virArchToString(i), end-start);
spaces around '-'
+}
/* QEMU Requires an emulator in the XML */
virCapabilitiesSetEmulatorRequired(caps);
@@ -2291,7 +2300,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
static int
qemuCapsInitQMP(qemuCapsPtr caps,
const char *libDir,
-const char *runDir,
uid_t runUid,
gid_t runGid)
{
@@ -2324,8 +2332,11 @@ qemuCapsInitQMP(qemuCapsPtr caps,
/* .pidfile suffix is used rather than .pid to avoid a possible clash
* with a qemu domain called capabilities
+ * Normally we'd use runDir for pid files, but because we're using
+ * -daemonize we need QEMU to be allowed to create them, rather
+ * than libvirtd. So we're using libDir which QEMU can write to
*/
-if (virAsprintf(pidfile, %s/%s, runDir, capabilities.pidfile) 0) {
+if (virAsprintf(pidfile, %s/%s, libDir, capabilities.pidfile) 0) {
Will this change work across an upgrade? That is, if I have a qemu
already running under libvirtd with the pid file in the old location,
and then restart to a newer libvirtd, do we ever try to read the pidfile
again, and fail because it is not in the right location?
+/*
+ * We explicitly need to use -daemonize here, rather than
+ * virCommandDaemonize, because we need to synchronize
+ * with QEMU creating its monitor socket API. Using
+ * daemonize guarnatees control won't return to libvirt
s/guarnatees/guarantees/
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix performance reliabilty of QMP probing
On 01/24/2013 01:34 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
This previous commit
commit 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9
Author: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
Date: Mon Nov 26 15:17:13 2012 +0100
qemu: Fix QMP Capabability Probing Failure
which attempted to make sure the QEMU process used for probing
ran as the right user id, caused serious performance regression
and unreliability in probing. The -daemonize switch in QEMU
guarantees that the monitor socket is present before the parent
process exits. This means libvirtd is guaranteed to be able to
connect immediately. By switching from -daemonize to the
virCommandDaemonize API libvirtd was no longer synchronized with
QEMU's startup process. The result was that the QEMU monitor
failed to open and went into its 200ms sleep loop. This happened
for all 25 binaries resulting in 5 seconds worth of sleeping
at libvirtd startup. In addition sometimes when libvirt connected,
QEMU would be partially initialized and crash causing total
failure to probe that binary.
This commit reverts the previous change, ensuring we do use the
-daemonize flag to QEMU. Startup delay is cut from 7 seconds
to 2 seconds on my machine, which is on a par with what it was
prior to the capabilities rewrite.
To deal with the fact that QEMU needs to be able to create the
pidfile, we switch pidfile location fron runDir to libDir, which
QEMU is guaranteed to be able to write to.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_capabilities.c | 50
++--
src/qemu/qemu_capabilities.h | 3 +--
2 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 95fa3da..703179d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -38,6 +38,7 @@
#include virbitmap.h
#include virnodesuspend.h
#include qemu_monitor.h
+#include virtime.h
#include sys/stat.h
#include unistd.h
@@ -916,11 +917,19 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache)
* so just probe for them all - we gracefully fail
* if a qemu-system-$ARCH binary can't be found
*/
-for (i = 0 ; i VIR_ARCH_LAST ; i++)
+unsigned long long a, b;
+for (i = 0 ; i VIR_ARCH_LAST ; i++) {
+unsigned long long start, end;
+if (virTimeMillisNow(start) 0)
+goto error;
if (qemuCapsInitGuest(caps, cache,
virArchFromHost(),
i) 0)
goto error;
+if (virTimeMillisNow(end) 0)
+goto error;
+VIR_DEBUG(Probed %s in %llums, virArchToString(i), end-start);
+}
Did you intend to leave in this debug code? (if you did, you need to
move the definition of a b up to the top of the block, and maybe
rename them to something more descriptive)
/* QEMU Requires an emulator in the XML */
virCapabilitiesSetEmulatorRequired(caps);
@@ -2291,7 +2300,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
static int
qemuCapsInitQMP(qemuCapsPtr caps,
const char *libDir,
-const char *runDir,
uid_t runUid,
gid_t runGid)
{
@@ -2324,8 +2332,11 @@ qemuCapsInitQMP(qemuCapsPtr caps,
/* .pidfile suffix is used rather than .pid to avoid a possible clash
* with a qemu domain called capabilities
+ * Normally we'd use runDir for pid files, but because we're using
+ * -daemonize we need QEMU to be allowed to create them, rather
+ * than libvirtd. So we're using libDir which QEMU can write to
*/
-if (virAsprintf(pidfile, %s/%s, runDir, capabilities.pidfile) 0) {
+if (virAsprintf(pidfile, %s/%s, libDir, capabilities.pidfile) 0) {
virReportOOMError();
goto cleanup;
}
@@ -2337,6 +2348,13 @@ qemuCapsInitQMP(qemuCapsPtr caps,
VIR_DEBUG(Try to get caps via QMP caps=%p, caps);
+/*
+ * We explicitly need to use -daemonize here, rather than
+ * virCommandDaemonize, because we need to synchronize
+ * with QEMU creating its monitor socket API. Using
+ * daemonize guarnatees control won't return to libvirt
s/guarnatees/guarantees/
+ * until the socket is present.
+ */
cmd = virCommandNewArgList(caps-binary,
-S,
-no-user-config,
@@ -2344,14 +2362,14 @@ qemuCapsInitQMP(qemuCapsPtr caps,
-nographic,
-M, none,
-qmp, monarg,
+ -pidfile, pidfile,
+ -daemonize,
NULL);
virCommandAddEnvPassCommon(cmd);
virCommandClearCaps(cmd);
hookData.runUid = runUid;
Re: [libvirt] [PATCH] Move QEMU capabilities initialization later in QEMU startup
On 01/24/2013 01:34 PM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently QEMU capabilities are initialized before the QEMU driver sets ownership on its various directories. The upshot is that if you change the user/group in the qemu.conf file, libvirtd will fail to probe QEMU the first time it is run after the config change. Moving QEMU capabilities initialization to after the chown() calls fixes this Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/qemu/qemu_driver.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.0.2
Daniel Veillard wrote: As scheduled and to gte a release by the end of the month next week, I have just tagged a release candidate 1 version in git and pushed associated tarballs and rpms to the usual place: ftp://libvirt.org/libvirt/ please give it a try and let's ocuse on fixing bugs untl the freeze end. I will probably make a release candidate 2 on Monday, and if everything goes well rol out the 1.0.2 release on Wednesday or Thursday. I know there are not many folks on this list using xen these days, but I'd like to get the series fixing libxl driver bugs and races into 1.0.2 if possible https://www.redhat.com/archives/libvir-list/2013-January/msg01463.html There are some fixes to libxl itself, which have now been committed to xen-unstable and backported to the xen-4.2 stable branch http://lists.xen.org/archives/html/xen-devel/2013-01/msg01941.html There are a few users besides myself hitting these bugs :) https://bugzilla.redhat.com/show_bug.cgi?id=893699 Thanks! Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] python: Fix bindings for virDomainSnapshotGet{Domain, Connect}
On Thu, Jan 24, 2013 at 11:26:35 -0700, Eric Blake wrote:
On 01/23/2013 04:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect()
wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed
to be ever implemented. The class should contain proper domain() and
connect() accessors that fetch python objects stored internally within
the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and
reimplements getDomain() and getConnect() methods as aliases to domain()
and connect() for backward compatibility.
---
python/generator.py | 4 +++-
python/libvirt-override-virDomainSnapshot.py | 8
src/libvirt.c| 10 --
3 files changed, 19 insertions(+), 3 deletions(-)
* virDomainSnapshotGetDomain:
* @snapshot: a snapshot object
*
- * Get the domain that a snapshot was created for
+ * Get the domain that a snapshot was created for.
+ *
Missing the following (based on copy-and-paste from virDomainGetConnect):
* Provides the domain pointer associated with a snapshot. The
* reference counter on the domain is not increased by this
* call.
You're right, it looks better this way.
...
ACK with those additions; no need to see a v3.
Thanks, I fixed the comments and pushed.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 01/10] locking: use virStrcpyStatic instead of memcpy
On 01/22/13 13:42, Peter Krempa wrote: On 01/18/13 14:50, John Ferlan wrote: --- src/locking/lock_driver_sanlock.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) If nobody else responds otherwise till tomorrow, I will push the patch as-is. Pushed. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] python: Fix bindings for virDomainSnapshotGet{Domain, Connect}
On 01/23/2013 06:26 AM, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect()
wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed
to be ever implemented. The class should contain proper domain() and
connect() accessors that fetch python objects stored internally within
the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and
reimplements getDomain() and getConnect() methods as aliases to domain()
and connect() for backward compatibility.
---
python/generator.py | 4 +++-
python/libvirt-override-virDomainSnapshot.py | 8
src/libvirt.c| 10 --
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/python/libvirt-override-virDomainSnapshot.py
b/python/libvirt-override-virDomainSnapshot.py
index 3da7bfd..bf708a5 100644
--- a/python/libvirt-override-virDomainSnapshot.py
+++ b/python/libvirt-override-virDomainSnapshot.py
@@ -1,3 +1,11 @@
+def getConnect(self):
+Get the connection that owns the domain that a snapshot was
created for
+return self.connect()
+
+def getDomain(self):
+Get the domain that a snapshot was created for
+return self.domain()
+
def listAllChildren(self, flags):
List all child snapshots and returns a list of snapshot objects
ret = libvirtmod.virDomainSnapshotListAllChildren(self._o, flags)
Not a big deal, but I think this chunk should be reverted. None of the other
classes provide getConnect, and it is inconsistent with how virDomainGetInfo()
is converted to virDomain.info()
Adding the doc string is useful certainly, but ideally it would be done for
all connect() impls.
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't share kerberos caches between domains
On 01/23/2013 08:26 PM, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=718377 complains that there were some SELinux AVCs when using vnc console over Kerberos. The root problem was that Kerberos tries to set up a cache file, and if we don't tell it where, then all domains use the same cache file, which violates sVirt protections. Setting the environment variable unconditionally should be safe, even for setups where Kerboros won't actually create a cache file. * src/qemu/qemu_process.c (qemuProcessStart): Set KRB5CACHEDIR for each domain. --- src/qemu/qemu_process.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 55d00e3..986e8ee 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1,7 +1,7 @@ /* * qemu_process.h: QEMU process management * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -3857,6 +3857,8 @@ int qemuProcessStart(virConnectPtr conn, virCommandSetPidFile(cmd, priv-pidfile); virCommandDaemonize(cmd); virCommandRequireHandshake(cmd); +virCommandAddEnvFormat(cmd, KRB5CACHEDIR=%s/%s.krb, + driver-cacheDir, vm-def-name); ret = virCommandRun(cmd, NULL); Thanks for taking a stab at this. The environment variable is actually called KRB5RCACHEDIR, and I don't think kerberos creates the directory for us. There's also KRB5RCACHENAME for pointing to a file path. What all this means is that someone should probably reproduce the bug first :) Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] snapshot: fix state after external snapshot of S3 domain
On 01/23/2013 08:28 PM, Osier Yang wrote: On 2013年01月24日 07:27, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=876829 complains that if a guest is put into S3 state (such as via virsh dompmsuspend) and then an external snapshot is taken, qemu forcefully transitions the domain to paused, but libvirt doesn't reflect that change internally. Thus, a user has to use 'virsh suspend' to get libvirt back in sync with qemu state, and if the user doesn't know this trick, then the guest appears hung. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateActiveExternal): Track fact that qemu wakes up a suspended domain on migration. --- /* we need to resume the guest only if it was previously running */ As you changed the codes, the comment needs to be changed too. Fixed... ACK with the comment fixed. and pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't share kerberos caches between domains
On 01/24/2013 03:53 PM, Cole Robinson wrote: On 01/23/2013 08:26 PM, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=718377 complains that there were some SELinux AVCs when using vnc console over Kerberos. The root problem was that Kerberos tries to set up a cache file, and if we don't tell it where, then all domains use the same cache file, which violates sVirt protections. Setting the environment variable unconditionally should be safe, even for setups where Kerboros won't actually create a cache file. +virCommandAddEnvFormat(cmd, KRB5CACHEDIR=%s/%s.krb, + driver-cacheDir, vm-def-name); ret = virCommandRun(cmd, NULL); Thanks for taking a stab at this. The environment variable is actually called KRB5RCACHEDIR, and I don't think kerberos creates the directory for us. There's also KRB5RCACHENAME for pointing to a file path. Good thing I haven't pushed yet. Where is this documented, so that I can fix my patch to match Kerberos expectations? What all this means is that someone should probably reproduce the bug first :) Unfortunately, I've got a huge learning curve ahead of me if I'm going to reproduce it (I was just implementing what looked like an easy fix based on the bugzilla content). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't share kerberos caches between domains
On 01/24/2013 07:05 PM, Eric Blake wrote: On 01/24/2013 03:53 PM, Cole Robinson wrote: On 01/23/2013 08:26 PM, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=718377 complains that there were some SELinux AVCs when using vnc console over Kerberos. The root problem was that Kerberos tries to set up a cache file, and if we don't tell it where, then all domains use the same cache file, which violates sVirt protections. Setting the environment variable unconditionally should be safe, even for setups where Kerboros won't actually create a cache file. +virCommandAddEnvFormat(cmd, KRB5CACHEDIR=%s/%s.krb, + driver-cacheDir, vm-def-name); ret = virCommandRun(cmd, NULL); Thanks for taking a stab at this. The environment variable is actually called KRB5RCACHEDIR, and I don't think kerberos creates the directory for us. There's also KRB5RCACHENAME for pointing to a file path. Good thing I haven't pushed yet. Where is this documented, so that I can fix my patch to match Kerberos expectations? I just looked at the krb5 code. What all this means is that someone should probably reproduce the bug first :) Unfortunately, I've got a huge learning curve ahead of me if I'm going to reproduce it (I was just implementing what looked like an easy fix based on the bugzilla content). Same reason why I never tested it and submitted the obvious patch :) - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: escape ipv6 for rbd network disk hosts
On 01/23/2013 08:20 PM, Osier Yang wrote:
On 2013年01月24日 10:15, Josh Durgin wrote:
Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
so they are often referenced by IP rather than hostname for
convenience, or to avoid relying on DNS. Using IPv4 addresses as the
host name works already, but IPv6 addresses require rbd-specific
escaping because the colon is used as an option separator in the
string passed to qemu.
Escape these colons, and enclose the IPv6 address in square brackets
if a port is specified.
Actually the IPv6 address is always enclosed in the code.
Indeed.
Signed-off-by: Josh Durginjosh.dur...@inktank.com
---
docs/schemas/domaincommon.rng |5 ++-
src/qemu/qemu_command.c| 34
+++
tests/qemuargv2xmltest.c |1 +
.../qemuxml2argv-disk-drive-network-rbd-ipv6.args |9 +
.../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 36
tests/qemuxml2argvtest.c |2 +
6 files changed, 79 insertions(+), 8 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml
diff --git a/docs/schemas/domaincommon.rng
b/docs/schemas/domaincommon.rng
index 7f3320e..273e54c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1099,7 +1099,10 @@
/attribute
/optional
attribute name=name
-ref name=dnsName/
+choice
+ref name=dnsName/
+ref name=ipAddr/
+/choice
/attribute
attribute name=port
ref name=unsignedInt/
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 02fe015..dfc042b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -34,6 +34,7 @@
#include virerror.h
#include virutil.h
#include virfile.h
+#include virstring.h
#include viruuid.h
#include c-ctype.h
#include domain_nwfilter.h
@@ -1937,13 +1938,16 @@ qemuBuildRBDString(virConnectPtr conn,
if (i) {
virBufferAddLit(opt, \\;);
}
-if (disk-hosts[i].port) {
-virBufferAsprintf(opt, %s\\:%s,
- disk-hosts[i].name,
- disk-hosts[i].port);
+
+/* assume host containing : is ipv6 */
+if (strchr(disk-hosts[i].name, ':')) {
+virBufferEscape(opt, '\\', :, [%s],
disk-hosts[i].name);
} else {
virBufferAsprintf(opt, %s, disk-hosts[i].name);
}
+if (disk-hosts[i].port) {
+virBufferAsprintf(opt, \\:%s, disk-hosts[i].port);
+}
}
}
@@ -1961,15 +1965,26 @@ error:
static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport)
{
char *port;
+size_t skip;
+char **parts;
disk-nhosts++;
if (VIR_REALLOC_N(disk-hosts, disk-nhosts) 0)
goto no_memory;
-port = strstr(hostport, \\:);
+if (strchr(hostport, ']')) {
+/* ipv6, strip brackets */1 / 384 =
+hostport += 1;
+port = strstr(hostport, ]\\:);
This can be simplified as (no need to get the same address
twice):
if ((port = strchr(hostport, ']'))) {
hostport += 1;
skip = 3;
} else {
...
}
Others looks pretty neat. ACK.
Good point, I'd forgotten that the port is mandatory when a name is
specified. Sending a v2.
Thanks!
Josh
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] qemu: escape ipv6 for rbd network disk hosts
Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
so they are often referenced by IP rather than hostname for
convenience, or to avoid relying on DNS. Using IPv4 addresses as the
host name works already, but IPv6 addresses require rbd-specific
escaping because the colon is used as an option separator in the
string passed to qemu.
Escape these colons, and enclose the IPv6 address in square brackets
so it is distinguished from the port, which is currently mandatory.
Acked-by: Osier Yang jy...@redhat.com
Signed-off-by: Josh Durgin josh.dur...@inktank.com
---
Since v1, in response to Osier's review:
- corrected commit message
- eliminated extra call to strstr() in qemuAddRBDHost
docs/schemas/domaincommon.rng |5 ++-
src/qemu/qemu_command.c| 33 ++
tests/qemuargv2xmltest.c |1 +
.../qemuxml2argv-disk-drive-network-rbd-ipv6.args |9 +
.../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 36
tests/qemuxml2argvtest.c |2 +
6 files changed, 78 insertions(+), 8 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7f3320e..273e54c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1099,7 +1099,10 @@
/attribute
/optional
attribute name=name
- ref name=dnsName/
+ choice
+ref name=dnsName/
+ref name=ipAddr/
+ /choice
/attribute
attribute name=port
ref name=unsignedInt/
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 02fe015..f6273c1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -34,6 +34,7 @@
#include virerror.h
#include virutil.h
#include virfile.h
+#include virstring.h
#include viruuid.h
#include c-ctype.h
#include domain_nwfilter.h
@@ -1937,13 +1938,16 @@ qemuBuildRBDString(virConnectPtr conn,
if (i) {
virBufferAddLit(opt, \\;);
}
-if (disk-hosts[i].port) {
-virBufferAsprintf(opt, %s\\:%s,
- disk-hosts[i].name,
- disk-hosts[i].port);
+
+/* assume host containing : is ipv6 */
+if (strchr(disk-hosts[i].name, ':')) {
+virBufferEscape(opt, '\\', :, [%s], disk-hosts[i].name);
} else {
virBufferAsprintf(opt, %s, disk-hosts[i].name);
}
+if (disk-hosts[i].port) {
+virBufferAsprintf(opt, \\:%s, disk-hosts[i].port);
+}
}
}
@@ -1961,15 +1965,25 @@ error:
static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport)
{
char *port;
+size_t skip;
+char **parts;
disk-nhosts++;
if (VIR_REALLOC_N(disk-hosts, disk-nhosts) 0)
goto no_memory;
-port = strstr(hostport, \\:);
+if ((port = strchr(hostport, ']'))) {
+/* ipv6, strip brackets */
+hostport += 1;
+skip = 3;
+} else {
+port = strstr(hostport, \\:);
+skip = 2;
+}
+
if (port) {
*port = '\0';
-port += 2;
+port += skip;
disk-hosts[disk-nhosts-1].port = strdup(port);
if (!disk-hosts[disk-nhosts-1].port)
goto no_memory;
@@ -1978,7 +1992,12 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char
*hostport)
if (!disk-hosts[disk-nhosts-1].port)
goto no_memory;
}
-disk-hosts[disk-nhosts-1].name = strdup(hostport);
+
+parts = virStringSplit(hostport, \\:, 0);
+if (!parts)
+goto no_memory;
+disk-hosts[disk-nhosts-1].name = virStringJoin((const char **)parts,
:);
+virStringFreeList(parts);
if (!disk-hosts[disk-nhosts-1].name)
goto no_memory;
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 2923324..e465f3d 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -185,6 +185,7 @@ mymain(void)
DO_TEST(disk-drive-network-nbd);
DO_TEST(disk-drive-network-gluster);
DO_TEST(disk-drive-network-rbd);
+DO_TEST(disk-drive-network-rbd-ipv6);
/* older format using CEPH_ARGS env var */
DO_TEST(disk-drive-network-rbd-ceph-env);
DO_TEST(disk-drive-network-sheepdog);
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
new file mode 100644
index 000..0c67229
---
Re: [libvirt] [PATCH 3/6] Introduce new XMLs to specify disk source using storage pool/vol
On 2013年01月25日 01:42, Laine Stump wrote: On 01/23/2013 10:24 AM, Osier Yang wrote: On 2013年01月23日 22:58, Daniel P. Berrange wrote: On Wed, Jan 23, 2013 at 10:55:25PM +0800, Osier Yang wrote: On 2013年01月23日 22:18, Daniel P. Berrange wrote: On Wed, Jan 23, 2013 at 07:04:35PM +0800, Osier Yang wrote: With this patch, one can specify the disk source using storage pool/volume like: disk type='file' device='disk' driver name='qemu' type='raw' cache='none'/ source type='pool' volume key='/var/lib/libvirt/images/foo.img'/ seclabel relabel='no'/ /source target dev='vdb' bus='virtio'/ /disk disk type='file' device='disk' driver name='qemu' type='raw' cache='none'/ source type='pool' volume path='/var/lib/libvirt/images/foo.img'/ seclabel relabel='no'/ /source target dev='vdb' bus='virtio'/ /disk disk type='file' device='disk' driver name='qemu' type='raw' cache='none'/ source type='pool' pool uuid|name=$var/ volume name='foo.img'/ seclabel relabel='no'/ /source target dev='vdb' bus='virtio'/ /disk If you're going to introduce new schema forsource, then you must introduce a new disk type value.ie a disk type='file'must always use thesource file='...'/ XML syntax, otherwise you cause backwards compatibility problems for applications Oh, yes. I need a v2. What you need here is adisk type='volume'/for your new schema. But before I make up the v2, do you see other design problem on the set? Thanks. I'm wondering if it is really requires to allow so many different options for specifyin the pool volume. Forinterface type='network' we were fine simply using the 'name' and ignoring UUID. I cna't help thinking that for storage we can similarly just use the pool name and volume name This was my hesitating too when on the half road. But to post the RFC earlier, and considering it's at least not a bad thing, as we provide all the interfaces, so I went on with it. I think it makes no big difference if we simply use pool name and volume name, but what I'm not sure is if the users will want the uuid for pool, and path/key for volume (using path/key is convenient as the pool is not even neccessary). (Keep in mind this is coming from a non-storage guy, so there may be some flaws in my logic :-) Rather clear actually. :-) Too many ways of describing the same thing is bad, as it leads to confusion. Also, since the point of making this abstraction is to isolate the host-specific details of the device from the domain's configuration, I think allowing the file path to be specified is counter-productive (since it could be different from host to host, depending on where an NFS share is mounted, for example). Right, I see several benefits of gluing the domain and storage: 1) the disk source (or source for other devices) can be not stable after a system rebooting, such as the path for LUN which behind a vHBA, and the scsi_host number. We can make them stabe via storage pool. Such as persistent vHBA support in storage pool. 2) As what you mentioned above: the same underlying disk source (for all of disk of type 'file', 'block', 'dir') can have different path on different host. Using the storage pool and volume can avoid this. 3) Simpler config for disk of 'network' type (in storage, it's pool of RBD, sheepdog, glusterfs type, well, we have to support the glusterfs pool in future), those network configurations can be taken from the pool/volume configuration. On the other hand, this avoid the duplication. It could implies more benifit I have not realizied yet, and thus more work to do though. Of course this is a bit of a different situation than network devices, since the pool/volume must end up pointing to the same bits from all hosts (either the exact same bits via a different access path, or a new copy of the bits migrated over to a different type of storage), but in the end it should be possible for the disk image to be in a local directory on one host, accessed by NFS on another, and maybe even via iscsi or lvm on another - those details should all be in the pool/volume definitions on each host, and the guest config should just say this disk's image is in pool x, volume y. So you agreed with just using the pool name and volume name? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: escape ipv6 for rbd network disk hosts
On 2013年01月25日 10:45, Josh Durgin wrote:
Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
so they are often referenced by IP rather than hostname for
convenience, or to avoid relying on DNS. Using IPv4 addresses as the
host name works already, but IPv6 addresses require rbd-specific
escaping because the colon is used as an option separator in the
string passed to qemu.
Escape these colons, and enclose the IPv6 address in square brackets
so it is distinguished from the port, which is currently mandatory.
Acked-by: Osier Yangjy...@redhat.com
Signed-off-by: Josh Durginjosh.dur...@inktank.com
---
Since v1, in response to Osier's review:
- corrected commit message
- eliminated extra call to strstr() in qemuAddRBDHost
docs/schemas/domaincommon.rng |5 ++-
src/qemu/qemu_command.c| 33 ++
tests/qemuargv2xmltest.c |1 +
.../qemuxml2argv-disk-drive-network-rbd-ipv6.args |9 +
.../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 36
tests/qemuxml2argvtest.c |2 +
6 files changed, 78 insertions(+), 8 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7f3320e..273e54c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1099,7 +1099,10 @@
/attribute
/optional
attribute name=name
-ref name=dnsName/
+choice
+ref name=dnsName/
+ref name=ipAddr/
+/choice
/attribute
attribute name=port
ref name=unsignedInt/
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 02fe015..f6273c1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -34,6 +34,7 @@
#include virerror.h
#include virutil.h
#include virfile.h
+#include virstring.h
#include viruuid.h
#include c-ctype.h
#include domain_nwfilter.h
@@ -1937,13 +1938,16 @@ qemuBuildRBDString(virConnectPtr conn,
if (i) {
virBufferAddLit(opt, \\;);
}
-if (disk-hosts[i].port) {
-virBufferAsprintf(opt, %s\\:%s,
- disk-hosts[i].name,
- disk-hosts[i].port);
+
+/* assume host containing : is ipv6 */
+if (strchr(disk-hosts[i].name, ':')) {
+virBufferEscape(opt, '\\', :, [%s], disk-hosts[i].name);
} else {
virBufferAsprintf(opt, %s, disk-hosts[i].name);
}
+if (disk-hosts[i].port) {
+virBufferAsprintf(opt, \\:%s, disk-hosts[i].port);
+}
}
}
@@ -1961,15 +1965,25 @@ error:
static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport)
{
char *port;
+size_t skip;
+char **parts;
disk-nhosts++;
if (VIR_REALLOC_N(disk-hosts, disk-nhosts) 0)
goto no_memory;
-port = strstr(hostport, \\:);
+if ((port = strchr(hostport, ']'))) {
+/* ipv6, strip brackets */
+hostport += 1;
+skip = 3;
+} else {
+port = strstr(hostport, \\:);
+skip = 2;
+}
+
if (port) {
*port = '\0';
-port += 2;
+port += skip;
disk-hosts[disk-nhosts-1].port = strdup(port);
if (!disk-hosts[disk-nhosts-1].port)
goto no_memory;
@@ -1978,7 +1992,12 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char
*hostport)
if (!disk-hosts[disk-nhosts-1].port)
goto no_memory;
}
-disk-hosts[disk-nhosts-1].name = strdup(hostport);
+
+parts = virStringSplit(hostport, \\:, 0);
+if (!parts)
+goto no_memory;
+disk-hosts[disk-nhosts-1].name = virStringJoin((const char **)parts,
:);
+virStringFreeList(parts);
if (!disk-hosts[disk-nhosts-1].name)
goto no_memory;
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 2923324..e465f3d 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -185,6 +185,7 @@ mymain(void)
DO_TEST(disk-drive-network-nbd);
DO_TEST(disk-drive-network-gluster);
DO_TEST(disk-drive-network-rbd);
+DO_TEST(disk-drive-network-rbd-ipv6);
/* older format using CEPH_ARGS env var */
DO_TEST(disk-drive-network-rbd-ceph-env);
DO_TEST(disk-drive-network-sheepdog);
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
new file mode 100644
index 000..0c67229
--- /dev/null
+++
Re: [libvirt] [PATCH] node_memory: Add '\n' to help message
On 2013年01月25日 03:40, Satoru Moriya wrote:
Linefeed is missed in the help of node-memory-tune.
This patch just adds '\n' to get a correct help message.
Signed-off-by: Satoru Moriyasatoru.mor...@hds.com
---
tools/virsh-host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c index d05e435..b83c893
100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -764,7 +764,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd
ATTRIBUTE_UNUSED)
static const vshCmdInfo info_node_memory_tune[] = {
{help, N_(Get or set node memory parameters)},
-{desc, N_(Get or set node memory parameters
+{desc, N_(Get or set node memory parameters\n
To get the memory parameters, use following command:
\n\n
virsh # node-memory-tune)},
{NULL, NULL}
--
1.7.11.7
ACK. And pushed since there is no chance to cause problem for
the upcoming build.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
On 24.01.2013 19:05, Eric Blake wrote: On 01/24/2013 02:41 AM, Michal Privoznik wrote: Currently, there is no reason to hold qemu driver locked throughout whole API execution. Moreover, we can use the new qemuDomObjFromDomain() internal API to lookup domain then. --- src/qemu/qemu_driver.c | 16 1 file changed, 4 insertions(+), 12 deletions(-) ACK. Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
