Re: [libvirt] [PATCH] usb: Correct test for virUSBDeviceListSteal

2013-07-05 Thread Laine Stump
On 07/05/2013 11:37 AM, Laine Stump wrote:
> On 07/05/2013 02:23 AM, Gonglei (Arei) wrote:
>> In the for loop, the if condition is always true, and will execute memmove.
>> But it will cause the list->devs[i+1] overflow while i equals list->count-1.
>>
>> Signed-off-by: Gonglei 
>> ---
>>  src/util/virusb.c |2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/util/virusb.c b/src/util/virusb.c
>> index d34e44f..30d0b12 100644
>> --- a/src/util/virusb.c
>> +++ b/src/util/virusb.c
>> @@ -497,7 +497,7 @@ virUSBDeviceListSteal(virUSBDeviceListPtr list,
>>  
>>  ret = list->devs[i];
>>  
>> -if (i != list->count--)
>> +if (i != --list->count)
>>  memmove(&list->devs[i],
>>  &list->devs[i+1],
>>  sizeof(*list->devs) * (list->count - i));
> This function is a good candidate for switching to VIR_DELETE_ELEMENT()
> instead. This will eliminate the bug that you found while making the
> code much shorter. I have a patch for that sitting around, I'll rebase
> it and post it.

Posted here:

  https://www.redhat.com/archives/libvir-list/2013-July/msg00336.html

(I forgot to mention it in that patch, but it assumes Michal's
"eliminate virReportOOMError()" patches have all been pushed.)

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


[libvirt] [PATCH] util: use VIR_(APPEND|DELETE)_ELEMENT for pci/usb device lists

2013-07-05 Thread Laine Stump
Eliminate memmove() by using VIR_*_ELEMENT API instead.

In both pci and usb cases, the count that held the size of the list
was unsigned int so it had to be changed to size_t.
---
This is an alternate fix to the same problem fixed by

  https://www.redhat.com/archives/libvir-list/2013-July/msg00324.html

(an off by one bug in the conditional in virUSBDeviceListSteal()).

This version fixes it by converting from open coded memmoves to using
VIR_(INSERT|DELETE)_ELEMENT.

 src/util/virpci.c | 18 +++---
 src/util/virusb.c | 26 +++---
 2 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index e42f6fa..b715122 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -80,7 +80,7 @@ struct _virPCIDevice {
 struct _virPCIDeviceList {
 virObjectLockable parent;
 
-unsigned int count;
+size_t count;
 virPCIDevicePtr *devs;
 };
 
@@ -1670,11 +1670,9 @@ virPCIDeviceListAdd(virPCIDeviceListPtr list,
 return -1;
 }
 
-if (VIR_REALLOC_N(list->devs, list->count+1) < 0)
+if (VIR_APPEND_ELEMENT(list->devs, list->count, dev, true) < 0)
 return -1;
 
-list->devs[list->count++] = dev;
-
 return 0;
 }
 
@@ -1723,17 +1721,7 @@ virPCIDeviceListStealIndex(virPCIDeviceListPtr list,
 return NULL;
 
 ret = list->devs[idx];
-
-if (idx != --list->count) {
-memmove(&list->devs[idx],
-&list->devs[idx + 1],
-sizeof(*list->devs) * (list->count - idx));
-}
-
-if (VIR_REALLOC_N(list->devs, list->count) < 0) {
-; /* not fatal */
-}
-
+VIR_DELETE_ELEMENT(list->devs, idx, list->count);
 return ret;
 }
 
diff --git a/src/util/virusb.c b/src/util/virusb.c
index 3bcb07c..103c301 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -60,7 +60,7 @@ struct _virUSBDevice {
 
 struct _virUSBDeviceList {
 virObjectLockable parent;
-unsigned int count;
+size_t count;
 virUSBDevicePtr *devs;
 };
 
@@ -451,11 +451,9 @@ virUSBDeviceListAdd(virUSBDeviceListPtr list,
 return -1;
 }
 
-if (VIR_REALLOC_N(list->devs, list->count+1) < 0)
+if (VIR_APPEND_ELEMENT(list->devs, list->count, dev, true) < 0)
 return -1;
 
-list->devs[list->count++] = dev;
-
 return 0;
 }
 
@@ -484,22 +482,12 @@ virUSBDeviceListSteal(virUSBDeviceListPtr list,
 int i;
 
 for (i = 0; i < list->count; i++) {
-if (list->devs[i]->bus != dev->bus ||
-list->devs[i]->dev != dev->dev)
-continue;
-
-ret = list->devs[i];
-
-if (i != list->count--)
-memmove(&list->devs[i],
-&list->devs[i+1],
-sizeof(*list->devs) * (list->count - i));
-
-if (VIR_REALLOC_N(list->devs, list->count) < 0) {
-; /* not fatal */
+if (list->devs[i]->bus == dev->bus &&
+list->devs[i]->dev == dev->dev) {
+ret = list->devs[i];
+VIR_DELETE_ELEMENT(list->devs, i, list->count);
+break;
 }
-
-break;
 }
 return ret;
 }
-- 
1.7.11.7

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


Re: [libvirt] [PATCH] usb: Correct test for virUSBDeviceListSteal

2013-07-05 Thread Laine Stump
On 07/05/2013 11:37 AM, Laine Stump wrote:
> On 07/05/2013 02:23 AM, Gonglei (Arei) wrote:
>> In the for loop, the if condition is always true, and will execute memmove.
>> But it will cause the list->devs[i+1] overflow while i equals list->count-1.


BTW, does that actually cause a failure? Although it's true that the 2nd
element of memmove will be 1 element past the end of the allocated
memory, the count of items to move in that case will be 0, so there
should never actually be any attempt to dereference the pointer. (Are
you maybe seeing a failure with valgrind or something?)


>>
>> Signed-off-by: Gonglei 
>> ---
>>  src/util/virusb.c |2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/util/virusb.c b/src/util/virusb.c
>> index d34e44f..30d0b12 100644
>> --- a/src/util/virusb.c
>> +++ b/src/util/virusb.c
>> @@ -497,7 +497,7 @@ virUSBDeviceListSteal(virUSBDeviceListPtr list,
>>  
>>  ret = list->devs[i];
>>  
>> -if (i != list->count--)
>> +if (i != --list->count)
>>  memmove(&list->devs[i],
>>  &list->devs[i+1],
>>  sizeof(*list->devs) * (list->count - i));
> This function is a good candidate for switching to VIR_DELETE_ELEMENT()
> instead. This will eliminate the bug that you found while making the
> code much shorter. I have a patch for that sitting around, I'll rebase
> it and post it.
>
> (I actually have 15 patches that replace memmove+VIR_REALLOC() with
> VIR_DELETE_ELEMENT and VIR_INSERT_ELEMENT. I had written them as
> examples of VIR_*_ELEMENT and posted them, but was too nervous of
> potential regression to push them (or even nag about reviews).
>
> --
> 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] add checking index for pci-bridge controller

2013-07-05 Thread Laine Stump
On 07/04/2013 07:10 AM, Jincheng Miao wrote:
> hi all,
>
> There is a bug here https://bugzilla.redhat.com/show_bug.cgi?id=981261 about 
> controller configuration.
> For the pci-bridge controller, if index less than zero, there is no alert for 
> it, and libvirt will pass this 
> value to qemu-kvm. But qemu-kvm will report the argument error, like:
> "qemu-kvm: -device pci-bridge,chassis_nr=-1,id=pci.-1,bus=pci.0,addr=0x9: 
> Parameter 'chassis_nr' expects uint8_t"
>
> So libvirt should check the arguments of controller.
>
> Here is my patch for pci-bridge:
>
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5693,6 +5693,14 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>   "have an address"));
>  goto error;
>  }
> +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
> +if (def->idx < 0) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("pci-bridge controller index invalid, 
> should not "
> + "less than zero"));
> +goto error;
> +}
> +

1) When sending a patch, please use git send-email, so that the patch is
in a form that can be directly applied to a local tree with "git am".

2) That seems like a reasonable check for the index of *any* controller,
not just pci-bridge. You could either put in a check right after the
call to virStrToLong_i(), or alternately call virStrToLong_ui() instead
(you would have to do it into a temporary variable, or change the
definition of idx to be unsigned int (will size_t work here?). I'm not
sure if there's a specific reason why idx was declared signed instead,
as I haven't examined the code in detail, but it's very likely we could
change it with no ill effect.

>  }
>
> And I also see qemuBuildControllerDevStr() in qemu_command.c 
> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
> if (def->idx == 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>_("PCI bridge index should be > 0"));
> goto error;
> }
> virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=pci.%d",
>   def->idx, def->idx);
>
> And if I modify it to "def->idx > 0", testsuite will fail.
> So why only check "def->idx == 0" ?  


This is specifically checking for index 0, because index 0 is reserved
for the implicit "pci-root" controller (soon to be "pcie-root" on q35
machine types) that is built into the virtual machine.

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

Re: [libvirt] attach-device error: XML error: unknown device type

2013-07-05 Thread Chris Evich
On 07/04/2013 03:43 AM, Michal Privoznik wrote:
> On 03.07.2013 20:34, Chris Evich wrote:
>> On 07/03/2013 09:46 AM, Daniel P. Berrange wrote:
>>> On Wed, Jul 03, 2013 at 09:44:46AM -0400, Chris Evich wrote:
 Hi,

 On Fedora 18 (libvirt 0.10.2.6-1) I'm trying to add a new serial device
 to a KVM VM with:

 virsh attach-device foobar /tmp/serial.xml

 and I keep getting:

 error: Failed to attach device from /tmp/serial.xml
 error: XML error: unknown device type

 with serial.xml:

 
   
   
 

 Though I tried it w/o the  tag, with and without the --config
 flag, and with and without the guest running.  Assuming it's similar, I
 tried and was successful in adding this device via virt-manager, though
 it complains if the VM is running (which is fine).

 What am I doing wrong with the virsh attach-device command or XML?
>>>
>>> There is no support for hotplug of any character device (that
>>> covers serial, parallel, console, channnel elements in the XML)
>>>
>>>
>>> Daniel
>>>
>>
>> Oops, sorry, should have been more clear than "and with and without the
>> guest running".
>>
>> * I see the 'unknown device type' error when guest is running and I run
>>   "virsh attach-device foobar /tmp/serial.xml --config".
>>
>> * I also get 'unknown device type' error when guest is NOT running, and
>>   I run "virsh attach" both with or without '--config' flag.
>>
>> * I see the 'unknown device type' error inside the "details" window
>>   when the guest is running and I use virt-manager.
>>
>> * I do NOT get any error, and the device is attached, when I use
>>   virt-manager, and the guest is NOT running (or by clicking yes
>>   after getting error in above bullet).
>>
>> The main difference in using virt-manager, is that I'm selecting the
>> file type and entering in the values manually instead of in an XML file.
> 
> This works as expected. Device attach implementation currently doesn't
> know how to handle  or any other character device. It doesn't
> matter if you're doing --live or --config hotplug. However, if you are
> trying virt-manager on a not running guest, then I guess virt-manager is
> just clever enough to redefine the domain and not call any hotplug API.
> This is equivalent to:
> 
> # virsh destroy $guest
> # virsh edit $guest (insert desired chardev XML snippet)
> # virsh start $guest
> 
> 
> Michal

Thanks.  So, possibly virt-manager is trying to do the attach-device
thing, failing, then using a different interface.  I guess that seems
plausible.  Though it would be nice if the libvirt docs were updated for
this command, explaining it only works for hot-puggable "hardware"
whether or not the --config option is used.

Also the 'unknown device type' error is certainly troublesome at best,
since it clearly does know what the type is, the XML is valid and should
parse.  In other words, the error is very different from the actual
underlying problem, which makes for much more difficult troubleshooting
and debugging.

Should I open some bugs on this, or would they just get *NTFIX immediately?

Thanks again.

-- 
Chris Evich, RHCA, RHCE, RHCDS, RHCSS
Quality Assurance Engineer
e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214

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


Re: [libvirt] [PATCH] usb: Correct test for virUSBDeviceListSteal

2013-07-05 Thread Laine Stump
On 07/05/2013 02:23 AM, Gonglei (Arei) wrote:
> In the for loop, the if condition is always true, and will execute memmove.
> But it will cause the list->devs[i+1] overflow while i equals list->count-1.
>
> Signed-off-by: Gonglei 
> ---
>  src/util/virusb.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/virusb.c b/src/util/virusb.c
> index d34e44f..30d0b12 100644
> --- a/src/util/virusb.c
> +++ b/src/util/virusb.c
> @@ -497,7 +497,7 @@ virUSBDeviceListSteal(virUSBDeviceListPtr list,
>  
>  ret = list->devs[i];
>  
> -if (i != list->count--)
> +if (i != --list->count)
>  memmove(&list->devs[i],
>  &list->devs[i+1],
>  sizeof(*list->devs) * (list->count - i));

This function is a good candidate for switching to VIR_DELETE_ELEMENT()
instead. This will eliminate the bug that you found while making the
code much shorter. I have a patch for that sitting around, I'll rebase
it and post it.

(I actually have 15 patches that replace memmove+VIR_REALLOC() with
VIR_DELETE_ELEMENT and VIR_INSERT_ELEMENT. I had written them as
examples of VIR_*_ELEMENT and posted them, but was too nervous of
potential regression to push them (or even nag about reviews).

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


Re: [libvirt] [libvirt-1.0.5]deadlock in child process after call function backtrace, any suggestions is appreciate!

2013-07-05 Thread Laine Stump
On 07/04/2013 01:43 AM, Caizhifeng wrote:
> Hi ALL,
>
> In order to catch the calltrace of deadlock in libvirtd, I modified the 
> function virMutexLock as follows:
>
> struct virMutex {
> pthread_mutex_t lock;
> void *trace[TRACE_SIZE];//added for test
> int ntrace; //added for test
> };
>
> void virMutexLock(virMutexPtr m)
> {
> struct timespec ts;
>
> if (0 == clock_gettime(CLOCK_REALTIME, &ts)) {
> ts.tv_sec += LOCK_TIMEOUT;
> if (pthread_mutex_timedlock(&m->lock, &ts) == ETIMEDOUT) {
> if (m->ntrace > 0)
> virLogBacktrace(m->ntrace, m->trace);
> pthread_mutex_lock(&m->lock);
> }
>
> m->ntrace = backtrace(m->trace, TRACE_SIZE);//record the 
> call trace information.
> } else {
> pthread_mutex_lock(&m->lock);
> }
> }
>
> The original is :
> void virMutexLock(virMutexPtr m){
> pthread_mutex_lock(&m->lock);
> }
>
> But, unfortunatly, sometimes, deadlock happened in child process after 
> virFork,

The problem is that backtrace() is not "async signal safe". The section
"Async-signal-safe functions" of "man 7 signal" explains what this is in
the context of a signal handler, but the conditions in a child process
just after fork() are really the same - a lock was acquired in one
thread of the parent, then *while that lock is being held* a different
thread of the parent calls fork(), which duplicates all of the process'
memory (including the lock) then creates a new process. In the new
process, the lock comes into existence marked as being held, but there
is no thread to unlock it, so when the child attempts to acquire the
lock, it waits forever.

To prevent this scenario, functions that aren't async signal safe
shouldn't be called in child processes of multithreaded parents (not
until a subsequent exec() has replaced the code that is running with
something new, that is).

I suppose you could solve this by putting a wrapper around backtrace()
that first acquires a lock that's visible to libvirt's code, then be
sure to separately acquire that lock just before fork(), and release it
in both the parent and child just after fork(). As an example, look at
the way that the logging lock is acquired/released just before/after
fork() in virFork(). Since your lock is being used for something that is
a part of libvirt's virMutex*() stuff, you'll of course need to use
lower level primitives.

(BTW, the virLog mutex is being acquired/released for exactly the same
reason - so that logging functions which aren't acync-signal safe can be
reliably called in the child process.)

>  the father libvirtd process' pid is 2987, and the child libvirtd process id 
> is 29509, which is forked in order to run a shell script.
>
> root@cvk143:~# ps -ef | grep libvirtd
> root  2987 1 52 08:36 ?00:40:38 /usr/sbin/libvirtd -d
> root 29509  2987  0 09:38 ?00:00:00 /usr/sbin/libvirtd -d
> root@cvk143:~#
>
> the child process's call trace is as follow:
> (gdb) bt
> #0  0x7f4d5e7fd89c in __lll_lock_wait () from 
> /lib/x86_64-linux-gnu/libpthread.so.0
> #1  0x7f4d5e7f9080 in _L_lock_903 () from 
> /lib/x86_64-linux-gnu/libpthread.so.0
> #2  0x7f4d5e7f8f19 in pthread_mutex_lock () from 
> /lib/x86_64-linux-gnu/libpthread.so.0
> #3  0x7f4d5e5607db in dl_iterate_phdr () from 
> /lib/x86_64-linux-gnu/libc.so.6
> #4  0x7f4d5c3c88b6 in _Unwind_Find_FDE () from 
> /lib/x86_64-linux-gnu/libgcc_s.so.1
> #5  0x7f4d5c3c5d70 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
> #6  0x7f4d5c3c6490 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
> #7  0x7f4d5c3c6d3e in _Unwind_Backtrace () from 
> /lib/x86_64-linux-gnu/libgcc_s.so.1
> #8  0x7f4d5e53b1c8 in backtrace () from /lib/x86_64-linux-gnu/libc.so.6
> #9  0x7f4d5f6f8c92 in virMutexLock (m=0x7f4d5fadedc0) at 
> util/virthreadpthread.c:128
> #10 0x7f4d5f6d0a3d in virLogLock () at util/virlog.c:152
> #11 0x7f4d5f6d0f66 in virLogReset () at util/virlog.c:311
> #12 0x7f4d5f6b3139 in virFork (pid=0x7f4d5a1a4310) at 
> util/vircommand.c:281
> #13 0x7f4d5f6b3b06 in virExec (cmd=0x7f4d2000a6f0) at 
> util/vircommand.c:493
> #14 0x7f4d5f6b8a34 in virCommandRunAsync (cmd=0x7f4d2000a6f0, pid=0x0) at 
> util/vircommand.c:2340
> #15 0x7f4d5f6b815c in virCommandRun (cmd=0x7f4d2000a6f0, 
> exitstatus=0x7f4d5a1a4728) at util/vircommand.c:2191
> #16 0x7f4d5f6b4bdd in virRun (argv=0x7f4d5a1a4730, status=0x7f4d5a1a4728) 
> at util/vircommand.c:776
> #17 0x7f4d60231006 in virStorageNFSPoolCheckSub (hostName=0x7f4d50011630 
> "192.168.0.6", hostDir=0x7f4d50014560 "/vms/isos")
> at storage/storage_backend.c:165
> #18 0x7f4d602312fa in virStoragePoolCheckFirst (pool=0x7f4d5000dcc0) at 
> storage/storage_backend.c:255
> #19 0x7f4d602383af in virStorageBackendFileSystemRefresh 
> (conn=0x7f4d3c0010a0, pool=0x7f4d5000dcc0) at storage/storage_backend_fs.c:887
> #20 0x

Re: [libvirt] [PATCH glib] Fix name of gvir_config_domain_chardev_source_pty_set_path

2013-07-05 Thread Christophe Fergeau
On Fri, Jun 28, 2013 at 03:49:21PM +0100, Daniel P. Berrange wrote:
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
> b/libvirt-gconfig/libvirt-gconfig.sym
> index ccddd88..f8c7cdd 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -66,7 +66,7 @@ LIBVIRT_GCONFIG_0.0.8 {
>   gvir_config_domain_chardev_source_pty_get_type;
>   gvir_config_domain_chardev_source_pty_new;
>   gvir_config_domain_chardev_source_pty_new_from_xml;
> - gvir_config_domain_source_pty_set_path;
> + gvir_config_domain_chardev_source_pty_set_path;

This is breaking ABI, not sure whether we should be more explicit about the
ABI breakage, or if we can silently fix it as we make no ABI stability
guarantees for now. Looks good otherwise.

Christophe


pgpBcvIifpw0E.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-users] libvirt & virtio_net - host.freeze@reset.domain

2013-07-05 Thread poma
On 04.07.2013 11:14, poma wrote:
> On 03.07.2013 13:43, Daniel P. Berrange wrote:
>> On Tue, Jul 02, 2013 at 01:25:21PM +0200, poma wrote:
>>> Hello people,
>>>
>>> libvirtd (libvirt) 1.0.5.2
>>> virsh 1.0.5.2
>>> virt-manager 0.10.0
>>>
>>> Host:
>>> Linux localhost 3.9.8-300.fc19.x86_64 #1 SMP Thu Jun 27 19:24:23 UTC
>>> 2013 x86_64 x86_64 x86_64 GNU/Linux
>>> Guest1:
>>> Linux localhost 3.9.8-300.fc19.i686.PAE #1 SMP Thu Jun 27 19:29:30 UTC
>>> 2013 i686 (none)
>>> Guest2:
>>> Linux localhost 3.9.8-300.fc19.x86_64 #1 SMP Thu Jun 27 19:24:23 UTC
>>> 2013 x86_64 x86_64 x86_64 GNU/Linux
>>>
>>>
>>> Virtual NIC - source & model:
>>> macvtap/NAT/bridge & virtio(virtio_net)
>>>
>>> Host freeze at "virsh reset " or "virt-manager - Force Reset"
>>> Need kernel.sysrq or power reset.
>>
>> I don't believe this is a libvirt issue - the 'virsh reset' command
>> will issue the 'system_reset' QEMU monitor command. This in turn
>> does an immediate reset of the guest CPUs/machine.
>>
>> Even if QEMU is doing the wrong thing, the kernel should obviously
>> never freeze/crash in this way - it should be robust against a
>> malicious QEMU process.
>>
>> You should probably send this message to the main QEMU and/or KVM
>> mailing lists so that it comes to the attention of people who are
>> more familiar with QEMU + virtio-net
>>
>>
>> Regards,
>> Daniel
>>
> 
> Thanks for your response.
> Mateusz hit the same issue[1] as well.
> OK, here we go.
> 
> 
> poma
> 
> 
> [1] https://lists.fedoraproject.org/pipermail/users/2013-July/436984.html

OK, is this a side effect or not, but certainly kernel[1] with
'bridge-timer-fix.patch'[2] resolves issue aforementioned, so far.
Thanks Cong, Josh.


poma


[1] http://koji.fedoraproject.org/koji/taskinfo?taskID=5569632
kernel-3.9.8-300.7.fc19.x86_64.rpm
[2] https://bugzilla.redhat.com/show_bug.cgi?id=880035#c53

Ref.
"fix for unreliable guest->host multicast triggers oops"
https://bugzilla.redhat.com/show_bug.cgi?id=980254

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


Re: [libvirt] LXC: autostart feature does set all interfaces to state up.

2013-07-05 Thread Richard Weinberger
Am 05.07.2013 03:36, schrieb Gao feng:
> On 07/05/2013 04:45 AM, Richard Weinberger wrote:
>> Hi,
>>
>> Am 03.07.2013 12:04, schrieb Gao feng:
>>> Hi,
>>> On 07/01/2013 03:45 PM, Richard Weinberger wrote:
 Hi!

 If you have multiple LXC containers with networking and the autostart 
 feature enabled libvirtd fails to
 up some veth interfaces on the host side.

 Most of the time only the first veth device is in state up, all others are 
 down.

 Reproducing is easy.
 1. Define a few containers (5 in my case)
 2. Run "virsh autostart ..." on each one.
 3. stop/start libvirtd

 You'll observe that all containers are running, but "ip a" will report on 
 the host
 side that not all veth devices are up and are not usable within the 
 containers.

 This is not userns related, just retested with libvirt of today.
>>>
>>> I can not reproduce this problem on my test bed...
>>
>> Strange.
>>
>>> maybe you should wait some seconds for the starting of these containers.
>>
>> Please see the attached shell script. Using it I'm able to trigger the issue 
>> on all of
>> my test machines.
>> run.sh creates six very minimal containers and enables autostart. Then it 
>> kills and restarts libvirtd.
>> After the script is done you'll see that only one or two veth devices are up.
>>
>> On the over hand, if I start them manually using a command like this one:
>> for cfg in a b c d e f ; do /opt/libvirt/bin/virsh -c lxc:/// start 
>> test-$cfg ; done
>> All veths are always up.
>>
> 
> 
> I still can not reproduce even use your script.
> 
> [root@Donkey-I5 Desktop]# ./run.sh
> Domain test-a defined from container_a.conf
> 
> Domain test-a marked as autostarted
> 
> Domain test-b defined from container_b.conf
> 
> Domain test-b marked as autostarted
> 
> Domain test-c defined from container_c.conf
> 
> Domain test-c marked as autostarted
> 
> Domain test-d defined from container_d.conf
> 
> Domain test-d marked as autostarted
> 
> Domain test-e defined from container_e.conf
> 
> Domain test-e marked as autostarted
> 
> Domain test-f defined from container_f.conf
> 
> Domain test-f marked as autostarted
> 
> 2013-07-05 01:26:47.155+: 27163: info : libvirt version: 1.1.0
> 2013-07-05 01:26:47.155+: 27163: debug : virLogParseOutputs:1334 : 
> outputs=1:file:/home/gaofeng/libvirtd.log
> waiting a bit
> 167: veth0:  mtu 1500 qdisc pfifo_fast 
> master virbr0 state UP qlen 1000
> 169: veth1:  mtu 1500 qdisc pfifo_fast 
> master virbr0 state UP qlen 1000
> 171: veth2:  mtu 1500 qdisc pfifo_fast 
> master virbr0 state UP qlen 1000
> 173: veth3:  mtu 1500 qdisc pfifo_fast 
> master virbr0 state UP qlen 1000
> 175: veth4:  mtu 1500 qdisc pfifo_fast 
> master virbr0 state UP qlen 1000
> 177: veth5:  mtu 1500 qdisc pfifo_fast 
> master virbr0 state UP qlen 1000
> 
> 
> Can you post your libvirt debug log?

Please see attached file.

43: veth0:  mtu 1500 qdisc pfifo_fast master 
virbr0 state UP qlen 1000
45: veth1:  mtu 1500 qdisc pfifo_fast master 
virbr0 state UP qlen 1000
47: veth2:  mtu 1500 qdisc pfifo_fast master virbr0 state 
DOWN qlen 1000
49: veth3:  mtu 1500 qdisc pfifo_fast master 
virbr0 state UP qlen 1000
51: veth4:  mtu 1500 qdisc pfifo_fast master 
virbr0 state UP qlen 1000
53: veth5:  mtu 1500 qdisc pfifo_fast master virbr0 state 
DOWN qlen 100

Thanks,
//richard



debug.log.bz2
Description: application/bzip
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/4] libxl: implement NUMA capabilities reporting

2013-07-05 Thread Dario Faggioli
[Moving the conversation on @xen-devel and adding Jan, as that seems
more appropriate]

[Jan, this came up as I'm implementing some NUMA bits in libvirt but, as
you see, the core of Jim's question is purely about Xen]

On lun, 2013-07-01 at 16:47 -0600, Jim Fehlig wrote:
> On my non-NUMA test machine I have the cell memory reported as
> 
>9175040
> 
Which is 8960, if divided by 1024, so at least it's consistent.

However...

> The machine has 8G of memory, running xen 4.3 rc6, with dom0_mem=1024M.  'xl 
> info --numa' says
> 
> total_memory   : 8190
> ...
> numa_info  :
> node:memsizememfreedistances
> 0:  8960   7116  10
> 
> Why is the node memsize > total_memory?

Mmm... Interesting question. I really never paid attention to this...
Jan (or anyone else), is that something known and/or expected?

I went checking this down in Xen, and here's what I found.

total_memory is: info.total_pages/((1 << 20) / vinfo->pagesize)

where 'info' is what libxl_get_physinfo() provides. On its turn,
libxl_get_physinfo() is xc_physinfo(), which is XEN_SYSCTL_physinfo,
which uses total_pages, which is assigned the number of pages, down in
__start_xen(), as it results from parsing the E820 map (looking for RAM
blocks).

OTOH, memsize comes from libxl_get_numainfo(), which is xc_numainfo(),
which is XEN_SYSCTL_numainfo, which puts in memsize what
node_spanned_pages() says.

That seems to come, on a NUMA box, from the parsing of SRAT, and on a
non-NUMA box, from just (start_pfn-end_pfn) (in pages, of course).

Anyway, on my NUMA box, I see something similar to what Jim sees on a
non-NUMA one:

# xl info -n
...
total_memory   : 12285
...
numa_info  :
node:memsizememfreedistances
   0:  6144 23  10,20
   1:  6720104  20,10

Where 6144+6720=12864 > 12285

Looking at what Xen says during boot, I see this (the [*], [+], [=] and
[|] are mine):

(XEN) Xen-e820 RAM map:
(XEN)   - 00096000 (usable)
(XEN)  000f - 0010 (reserved)   [*]
(XEN)  0010 - dbdf9c00 (usable)
(XEN)  dbdf9c00 - dbe4bc00 (ACPI NVS)   [+]
(XEN)  dbe4bc00 - dbe4dc00 (ACPI data)  [=]
(XEN)  dbe4dc00 - dc00 (reserved)   [|]
(XEN)  f800 - fd00 (reserved)
(XEN)  fe00 - fed00400 (reserved)
(XEN)  fee0 - fef0 (reserved)
(XEN)  ffb0 - 0001 (reserved)
(XEN)  0001 - 00032400 (usable)
...
(XEN) System RAM: 12285MB (12580412kB)

And my math says that 12285MB is the sum of the areas marked as
(usable), i.e., I guess, what during parsing is recognised as
E820_RAM... which makes total sense.

A bit below that I have this:

(XEN) SRAT: Node 1 PXM 1 0-dc00
(XEN) SRAT: Node 1 PXM 1 1-1a400
(XEN) SRAT: Node 0 PXM 0 1a400-32400

Which, after the needed calculations, gives exactly the same results
than memsize-s in `xl info -n'.

Now, if I add up the [*], [+], [=] and [|] regions above, and then
subtract that from node 1's PXMs, I see that node 1 has only ~6141MB of
usable RAM, instead of 6720MB.

And in fact, 6720-6141=579, just as much as 12864-12285=579.

So, if I haven't messed up with the calculations, it looks like that
Xen, when reporting to the upper layers the amount of memory it has
available, does filter out the non-RAM regions, if this happens via
XEN_SYSCTL_physinfo (i.e., by parsing E820), while it does not do that,
if this happens via XEN_SYSCTL_numainfo (i.e., by parsing ACPI SRAT).

What I'm not sure about is whether or not that was something
known/intended and whether or not it is something we should be concerned
about.

Thanks and Regards,
Dario

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



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

Re: [libvirt] [PATCH] Don't spam logs with "port 0 must be in range" errors

2013-07-05 Thread Daniel P. Berrange
On Fri, Jul 05, 2013 at 09:46:26AM +0200, Michal Privoznik wrote:
> On 04.07.2013 21:46, Jiri Denemark wrote:
> > Whenever virPortAllocatorRelease is called with port == 0, it complains
> > that the port is not in an allowed range, which is expectable as the
> > port was never allocated. Let's make virPortAllocatorRelease ignore 0
> > ports in a similar way free() ignores NULL pointers.
> > ---
> >  src/qemu/qemu_migration.c   |  4 ++--
> >  src/qemu/qemu_process.c | 27 +++
> >  src/util/virportallocator.c |  4 
> >  3 files changed, 17 insertions(+), 18 deletions(-)
> 
> Since making an application to listen on port 0 on Linux is almost
> impossible I feel confident enough to give ACK.

s/almost//

If you specify a port==0 in a listen() call, the kernel will
auto-allocate a random non-zero port for you.

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] Don't spam logs with "port 0 must be in range" errors

2013-07-05 Thread Michal Privoznik
On 04.07.2013 21:46, Jiri Denemark wrote:
> Whenever virPortAllocatorRelease is called with port == 0, it complains
> that the port is not in an allowed range, which is expectable as the
> port was never allocated. Let's make virPortAllocatorRelease ignore 0
> ports in a similar way free() ignores NULL pointers.
> ---
>  src/qemu/qemu_migration.c   |  4 ++--
>  src/qemu/qemu_process.c | 27 +++
>  src/util/virportallocator.c |  4 
>  3 files changed, 17 insertions(+), 18 deletions(-)

Since making an application to listen on port 0 on Linux is almost
impossible I feel confident enough to give ACK.

Michal

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