Re: [libvirt] [PATCH v2 1/8] Added public API to enable post-copy migration

2014-11-05 Thread Andrea Arcangeli
Hi everyone,

On Thu, Nov 06, 2014 at 09:18:04AM +0200, Cristian Klein wrote: I
> talked to the qemu post-copy guys (Andrea and Dave in CC). Starting
> post-copy immediately is a bad performance choice: The VM will start
> on the destination hypervisor before the read-only or kernel memory
> is there. This means that those pages need to be pulled on-demand,
> hence a lot of overhead and interruptions in the VM’s execution.
> 
> Instead, it is better to first do one pass of pre-copy and only then
> trigger post-copy. In fact, I did an experiment with a video
> streaming VM and starting post-copy after the first pass of pre-copy
> (instead of starting post-copy immediately) reduces downtime from
> 3.5 seconds to under 1 second.
> 
> Given all above, I propose the following post-copy API in libvirt:
> 
> virDomainMigrateXXX(..., VIR_MIGRATE_ENABLE_POSTCOPY)
> virDomainMigrateStartPostCopy(...) // from a different thread
> 
> This is for those who just need the post-copy mechanism and want to
> implement a policy themselves.
> 
> 
> virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY)
> 
> This is for those who want to use post-copy without caring about any
> low-level details, offering a good enough policy for most cases.
> 
> What do you think? Would you accept patches that implement this API?

I agree, even better would be to also pass a parameter to specify how
many passes of precopy to run before engaging postcopy, adding at
least the number of passes parameter shouldn't be a huge change and in
doubt you can just define it to 1.

The other things needed are:

1) adding an event that doesn't require libvirt to poll to know when
   the source node has been stopped (then if postcopy was engaged
   precopy may not have finished, but the problem remains the same as
   with pure precopy: we need to know efficiently when exactly the
   source node has been stopped without adding an average 25msec
   latency)

2) preparing a second socket for qemu so we can run the out of band
   requests of postcopy without incurring into artificial latencies
   created by the socket sendbuffer kept filled by the background
   transfer (the hack to decrease /proc/sys/net/ipv4/tcp_wmem helps
   tremendously, but it's unlikely to ever be as efficient as having
   two sockets, potentially both running on openssl etc..)

Thanks,
Andrea

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

Re: [libvirt] [PATCH] CVE-2014-7823: dumpxml: security hole with migratable flag

2014-11-05 Thread Eric Blake
On 11/05/2014 05:30 PM, Eric Blake wrote:
> Commit 28f8dfd (v1.0.0) introduced a security hole: in at least
> the qemu implementation of virDomainGetXMLDesc, the use of the
> flag VIR_DOMAIN_XML_MIGRATABLE (which is usable from a read-only
> connection) triggers the implicit use of VIR_DOMAIN_XML_SECURE
> prior to calling qemuDomainFormatXML.  However, the use of
> VIR_DOMAIN_XML_SECURE is supposed to be restricted to read-write
> clients only.  This patch treats the migratable flag as requiring
> the same permissions, rather than analyzing what might break if
> migratable xml no longer includes secret information.
> 
> Fortunately, the information leak is low-risk: all that is gated
> by the VIR_DOMAIN_XML_SECURE flag is the VNC connection password;
> but VNC passwords are already weak (FIPS forbids their use, and
> on a non-FIPS machine, anyone stupid enough to trust a max-8-byte
> password sent in plaintext over the network deserves what they
> get).  SPICE offers better security than VNC, and all other
> secrets are properly protected by use of virSecret associations
> rather than direct output in domain XML.
> 
> * src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_GET_XML_DESC):
> Tighten rules on use of migratable flag.
> * src/libvirt-domain.c (virDomainGetXMLDesc): Likewise.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> The libvirt-security list agreed that this did not need an embargo
> because it is low-risk; but I'm on the road this week, so while
> this patch for master can go in now, I won't complete the backport
> to all the affected stable branches (everything since v1.0.0) or
> do the Libvirt Security Notice writeup until Monday.

Pushed based on positive review on the libvirt-security list.

-- 
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 1/8] Added public API to enable post-copy migration

2014-11-05 Thread Cristian Klein
On 01 Oct 2014, at 12:07 , Jiri Denemark  wrote:

> On Wed, Oct 01, 2014 at 10:45:33 +0200, Cristian KLEIN wrote:
>> On 2014-09-30 17:16, Daniel P. Berrange wrote:
>>> On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote:
 On Tue, Sep 30, 2014 at 16:39:22 +0200, Cristian Klein wrote:
> Signed-off-by: Cristian Klein 
> ---
>  include/libvirt/libvirt.h.in | 1 +
>  src/libvirt.c| 7 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 5217ab3..82f3aeb 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1225,6 +1225,7 @@ typedef enum {
>  VIR_MIGRATE_ABORT_ON_ERROR= (1 << 12), /* abort migration on I/O 
> errors happened during migration */
>  VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */
>  VIR_MIGRATE_RDMA_PIN_ALL  = (1 << 14), /* RDMA memory pinning */
> +VIR_MIGRATE_POSTCOPY  = (1 << 15), /* enable (but don't 
> start) post-copy */
>  } virDomainMigrateFlags;
 
 I still think we should add an extra flag to start post copy
 immediately. To address your concerns about it, I don't think it's
 implementing a policy in libvirt. It's for apps that want to make sure
 migration converges without having to spawn another thread and monitor
 the progress or wait for a timeout. It's a bit similar to migrating a
 paused domain vs. migrating a running domain and pausing it when it
 doesn't seem to converge.
>>> 
>>> Your point about spawning another thread makes me wonder if we should
>>> actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require
>>> P2P migration of course). If this flag were set, virDomainMigrateXXX would
>>> only block for long enough to start the migration and then return.
>>> 
>>> Callers can use the job info API to monitor progress & success/failure.
>>> 
>>> Then we wouldn't have to keep adding flags like you suggest - apps can
>>> just easily call the appropriate API right away with no threads needed
>> 
>> This would make a lot of sense. The user would call:
>> 
>> """
>> virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY | VIR_MIGRATE_ASYNC)
>> virDomainMigrateStartPostCopy(...)
>> """
>> 
>> Would this be seen as more cumbersome than having a dedicated 
>> VIR_MIGRATE_POSTCOPY_AUTOSTART?
> 
> The ASYNC flag Daniel suggested makes sense, so I guess you can just
> ignore my request for a special flag. Although, I don't think the ASYNC
> stuff needs to be done within this series, let's just focus on the
> post-copy stuff.

Hi Jirka,

I talked to the qemu post-copy guys (Andrea and Dave in CC). Starting post-copy 
immediately is a bad performance choice: The VM will start on the destination 
hypervisor before the read-only or kernel memory is there. This means that 
those pages need to be pulled on-demand, hence a lot of overhead and 
interruptions in the VM’s execution.

Instead, it is better to first do one pass of pre-copy and only then trigger 
post-copy. In fact, I did an experiment with a video streaming VM and starting 
post-copy after the first pass of pre-copy (instead of starting post-copy 
immediately) reduces downtime from 3.5 seconds to under 1 second.

Given all above, I propose the following post-copy API in libvirt:

virDomainMigrateXXX(..., VIR_MIGRATE_ENABLE_POSTCOPY)
virDomainMigrateStartPostCopy(...) // from a different thread

This is for those who just need the post-copy mechanism and want to implement a 
policy themselves.


virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY)

This is for those who want to use post-copy without caring about any low-level 
details, offering a good enough policy for most cases.

What do you think? Would you accept patches that implement this API?

Cristian

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


Re: [libvirt] [PATCH 3/3] qemuxml2argvmock: Mock virNumaNodesetIsAvailable

2014-11-05 Thread Martin Kletzander

On Wed, Nov 05, 2014 at 06:07:18PM +0100, Michal Privoznik wrote:

And yet again one fix of 90286418. The problem is, if libvirt is
built with NUMA disabled (--without-numad --without-numactl), the
testsuite doesn't count on that and some tests fail. This is
because the virNumaNodesetIsAvailable() function is taken from
another section of virnuma.c which contains functions stubs for
case NUMA is disabled at build time. And the function basically
yields an error. However, we want our testsuite to be
deterministic and hence we ought to mock the function.

Signed-off-by: Michal Privoznik 
---
tests/qemuxml2argvmock.c | 9 +
1 file changed, 9 insertions(+)

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 7218747..7b8bdd6 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -39,3 +39,12 @@ virNumaGetMaxNode(void)

   return maxnodesNum;
}
+
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune);
+int maxHostNode = virNumaGetMaxNode();
+
+return maxGuestNode <= maxHostNode;
+}
--
2.0.4



NACK, we shouldn't need to mock this function.  I went over the commit
once more and I've realized it doesn't support non-contiguous nodesets
even if it could.  And since it will use only functions that have both
versions (better one with NUMA support and a stub without it) it can
be moved out of those #ifdefs.  I have the patch almost ready :)

I'll also remove the need for util to have connection to conf (it will
make more sense, hopefully.

Martin


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

Re: [libvirt] [PATCH 3/3] qemuxml2argvmock: Mock virNumaNodesetIsAvailable

2014-11-05 Thread Peter Krempa
On 11/05/14 18:07, Michal Privoznik wrote:
> And yet again one fix of 90286418. The problem is, if libvirt is
> built with NUMA disabled (--without-numad --without-numactl), the
> testsuite doesn't count on that and some tests fail. This is
> because the virNumaNodesetIsAvailable() function is taken from
> another section of virnuma.c which contains functions stubs for
> case NUMA is disabled at build time. And the function basically
> yields an error. However, we want our testsuite to be
> deterministic and hence we ought to mock the function.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  tests/qemuxml2argvmock.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> index 7218747..7b8bdd6 100644
> --- a/tests/qemuxml2argvmock.c
> +++ b/tests/qemuxml2argvmock.c
> @@ -39,3 +39,12 @@ virNumaGetMaxNode(void)
>  
> return maxnodesNum;
>  }
> +
> +bool
> +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
> +{
> +int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune);
> +int maxHostNode = virNumaGetMaxNode();
> +
> +return maxGuestNode <= maxHostNode;
> +}
> 

The reason the function needs to be in the conditionally compiled
section is that the function uses the NUMA_NUM_NODES macro. The rest of
the used symbols has a stub and/or is already mocked. So we need to mock
this too unfortunately.

ACK

Peter



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

Re: [libvirt] [PATCH 2/3] private.syms: Export virDomainNumatuneSpecifiedMaxNode

2014-11-05 Thread Peter Krempa
On 11/05/14 18:07, Michal Privoznik wrote:
> As of 90286418 the function is introduced. However, it's missing
> an entry in the libvirt_private.syms so it can't be mocked.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/libvirt_private.syms | 1 +
>  1 file changed, 1 insertion(+)
> 

ACK,

Peter




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

Re: [libvirt] [PATCH 1/3] virnuma: Add some more comments

2014-11-05 Thread Peter Krempa
On 11/05/14 18:07, Michal Privoznik wrote:
> Especially add comments to mark ifdef, else and endif sections.

'Especially' sounds a bit odd here since the comments you are adding are
only else/ifdef macros.

> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virnuma.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

ACK

Peter




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 0/3] Libvirt memory & NUMA fixes

2014-11-05 Thread Prerna Saxena

On Wednesday 05 November 2014 08:40 PM, Michal Privoznik wrote:
> On 05.11.2014 11:56, Prerna Saxena wrote:
>> This patch set addresses a bunch of memory & NUMA fixes.
>>
>>
>> Series Description:
>> ===
>> Patch 1/3 : Use consistent data type to represent memory elements in various 
>> XML attributes. This ensures all memory elements are always represented as 
>> 'unsigned long long'.
>> Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of 
>> a NUMA cell. This enables users to easily describe how much memory needs to 
>> be allocated to each NUMA cell for a guest
>> domain.
>> Patch 3/3 : This augments test cases to add the 'unit' tag.
>>
>> Regards,
>>
> The 2/3 and 3/3 should be merged together so that 'make check' won't fail 
> after 2/3. Otherwise looking good. I've merged 1/3 already.
>

Thanks Michal !
I'll merge the remaining two and send out a cumulative patch :)

Regards,

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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


Re: [libvirt] [PATCHv2 1/2] qemu: revert patch - bandwidth tuning in session mode

2014-11-05 Thread Michal Privoznik

On 05.11.2014 18:41, Erik Skultety wrote:

Since there was a valid note to patch 43b67f2e about the best spot to
check for bandwidth set call while having libvirt daemon run in session
mode, this patch reverts previous changes dealing with bandwith
(also reverts adding variable @cfg in qemuDomainGetNumaParameters which
  does not have any use at the moment, but getting and unreferencing
  driver's config) in qemu_driver.c and qemu_command.c. There will be
another patch in the series which introduces the fix itself.
---
  src/qemu/qemu_command.c | 11 ---
  src/qemu/qemu_driver.c  |  9 -
  2 files changed, 20 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 917639e..956bb14 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7841,17 +7841,6 @@ qemuBuildCommandLine(virConnectPtr conn,
 _("CPU tuning is not available in session mode"));
  goto error;
  }
-
-virDomainNetDefPtr *nets = def->nets;
-virNetDevBandwidthPtr bandwidth = NULL;
-size_t nnets = def->nnets;
-for (i = 0; i < nnets; i++) {
-if ((bandwidth = virDomainNetGetActualBandwidth(nets[i])) != NULL) 
{
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-_("Network bandwidth tuning is not available in session 
mode"));
-goto error;
-}
-}
  }

  for (i = 0; i < def->ngraphics; ++i) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6acaea8..edd82c1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9383,7 +9383,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
  size_t i;
  virDomainObjPtr vm = NULL;
  virDomainDefPtr persistentDef = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
  char *nodeset = NULL;
  int ret = -1;
  virCapsPtr caps = NULL;
@@ -9402,7 +9401,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
  return -1;

  priv = vm->privateData;
-cfg = virQEMUDriverGetConfig(driver);

  if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
  goto cleanup;
@@ -9476,7 +9474,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
  if (vm)
  virObjectUnlock(vm);
  virObjectUnref(caps);
-virObjectUnref(cfg);
  return ret;
  }

@@ -10447,12 +10444,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
  if (virDomainSetInterfaceParametersEnsureACL(dom->conn, vm->def, flags) < 
0)
  goto cleanup;

-if (!cfg->privileged) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-   _("Network bandwidth tuning is not available in session 
mode"));
-goto cleanup;
-}
-
  if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
  goto cleanup;




ACK, however, I'm not pushing this one until 2/2 is fixed.

Michal

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


Re: [libvirt] [PATCHv2 2/2] Iface: disallow network tuning in session mode globally

2014-11-05 Thread Michal Privoznik

On 05.11.2014 18:41, Erik Skultety wrote:

Patch 43b67f2e disallowed network tuning only with qemu driver, however
this patch moved the check for root privileges into
virNetDevBandwidthSet function, so the call should now
fail in all possible cases. A mock function was created so that the test
suite doesn't fail because of unsufficient privileges.
---
  src/util/virnetdevbandwidth.c  |  8 
  tests/Makefile.am  | 11 ++-
  tests/virnetdevbandwidthtest.c | 14 +-
  3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 5fa231a..8360fd4 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -27,6 +27,7 @@
  #include "viralloc.h"
  #include "virerror.h"
  #include "virstring.h"
+#include "unistd.h"

  #define VIR_FROM_THIS VIR_FROM_NONE

@@ -74,6 +75,13 @@ virNetDevBandwidthSet(const char *ifname,
  goto cleanup;
  }

+if (geteuid() != 0) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("Network bandwidth tuning is not available"
+ " in session mode"));
+return -1;
+}
+
  virNetDevBandwidthClear(ifname);

  if (bandwidth->in && bandwidth->in->average) {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7b22f90..7fa4575 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -392,6 +392,7 @@ test_libraries = libshunload.la \
virnetserverclientmock.la \
vircgroupmock.la \
virpcimock.la \
+   virnetdevbandwidthmock.la \
$(NULL)
  if WITH_QEMU
  test_libraries += libqemumonitortestutils.la \
@@ -409,7 +410,9 @@ test_libraries += \
  endif WITH_DBUS

  if WITH_LINUX
-test_libraries += virusbmock.la
+test_libraries += \
+   virusbmock.la
+   $(NULL)
  endif WITH_LINUX



This chunk seem unrelated.


  if WITH_TESTS
@@ -829,6 +832,12 @@ virnetdevbandwidthtest_SOURCES = \
virnetdevbandwidthtest.c testutils.h testutils.c
  virnetdevbandwidthtest_LDADD = $(LDADDS) $(LIBXML_LIBS)

+virnetdevbandwidthmock_la_SOURCES = \
+   virnetdevbandwidthmock.c
+virnetdevbandwidthmock_la_CFLAGS = $(AM_CFLAGS)
+virnetdevbandwidthmock_la_LDFLAGS = -module -avoid-version \
+-rpath /evil/libtool/hack/to/force/shared/lib/creation
+


Did you forget to 'git add virnetdevbandwidthmock.c'? It's not in the 
patch anywhere.



  virkmodtest_SOURCES = \
virkmodtest.c testutils.h testutils.c
  virkmodtest_LDADD = $(LDADDS)
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 384991e..df69bac 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -21,6 +21,9 @@
  #include 

  #include "testutils.h"
+
+#ifdef WITH_LINUX
+
  #define __VIR_COMMAND_PRIV_H_ALLOW__
  #include "vircommandpriv.h"
  #include "virnetdevbandwidth.h"
@@ -167,4 +170,13 @@ mymain(void)
  return ret;
  }

-VIRT_TEST_MAIN(mymain);
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevbandwidthmock.so")
+
+#else
+
+int main(void)
+{
+return EXIT_AM_SKIP;
+}
+
+#endif /* WITH_LINUX */



Okay, we can make this test run on Linux only. Well, I don't think they 
have tc in *BSD anyway, do they?


Michal

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


Re: [libvirt] [PATCH] UpdateDevice: Allow startupPolicy update

2014-11-05 Thread Michal Privoznik

On 05.11.2014 16:29, Peter Krempa wrote:

Add qemu: prefix to subject and mention that it's only with the _CONFIG
flag.


Fixed.



On 11/05/14 14:11, Michal Privoznik wrote:

Users might want to update startupPolicy via the
virDomainUpdateDeviceFlags API too. This patch
implements the feature on config layer.

Signed-off-by: Michal Privoznik 
---
  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 6acaea8..6fc15c0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7437,6 +7437,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
  if (disk->src->format)
  orig->src->format = disk->src->format;
  disk->src->path = NULL;
+orig->startupPolicy = disk->startupPolicy;


Looks on par with what we are doing now. I wanted to comment that if you
don't specify a startup policy you might want to keep the existing one,
but the code does not use this approach, thus ...


Well, in order to do that we would need to allow the following:

   startupPolicy='default'

to allow users to remove already set startupPolicy. But that's not 
supported currently, so I went the other way.





  break;

  case VIR_DOMAIN_DEVICE_NET:



ACK with the subject tweaked





Pushed now. Thanks.

Michal

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


[libvirt] [PATCHv2 2/2] Iface: disallow network tuning in session mode globally

2014-11-05 Thread Erik Skultety
Patch 43b67f2e disallowed network tuning only with qemu driver, however
this patch moved the check for root privileges into
virNetDevBandwidthSet function, so the call should now
fail in all possible cases. A mock function was created so that the test
suite doesn't fail because of unsufficient privileges.
---
 src/util/virnetdevbandwidth.c  |  8 
 tests/Makefile.am  | 11 ++-
 tests/virnetdevbandwidthtest.c | 14 +-
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 5fa231a..8360fd4 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -27,6 +27,7 @@
 #include "viralloc.h"
 #include "virerror.h"
 #include "virstring.h"
+#include "unistd.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -74,6 +75,13 @@ virNetDevBandwidthSet(const char *ifname,
 goto cleanup;
 }
 
+if (geteuid() != 0) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("Network bandwidth tuning is not available"
+ " in session mode"));
+return -1;
+}
+
 virNetDevBandwidthClear(ifname);
 
 if (bandwidth->in && bandwidth->in->average) {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7b22f90..7fa4575 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -392,6 +392,7 @@ test_libraries = libshunload.la \
virnetserverclientmock.la \
vircgroupmock.la \
virpcimock.la \
+   virnetdevbandwidthmock.la \
$(NULL)
 if WITH_QEMU
 test_libraries += libqemumonitortestutils.la \
@@ -409,7 +410,9 @@ test_libraries += \
 endif WITH_DBUS
 
 if WITH_LINUX
-test_libraries += virusbmock.la
+test_libraries += \
+   virusbmock.la
+   $(NULL)
 endif WITH_LINUX
 
 if WITH_TESTS
@@ -829,6 +832,12 @@ virnetdevbandwidthtest_SOURCES = \
virnetdevbandwidthtest.c testutils.h testutils.c
 virnetdevbandwidthtest_LDADD = $(LDADDS) $(LIBXML_LIBS)
 
+virnetdevbandwidthmock_la_SOURCES = \
+   virnetdevbandwidthmock.c
+virnetdevbandwidthmock_la_CFLAGS = $(AM_CFLAGS)
+virnetdevbandwidthmock_la_LDFLAGS = -module -avoid-version \
+-rpath /evil/libtool/hack/to/force/shared/lib/creation
+
 virkmodtest_SOURCES = \
virkmodtest.c testutils.h testutils.c
 virkmodtest_LDADD = $(LDADDS)
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 384991e..df69bac 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -21,6 +21,9 @@
 #include 
 
 #include "testutils.h"
+
+#ifdef WITH_LINUX
+
 #define __VIR_COMMAND_PRIV_H_ALLOW__
 #include "vircommandpriv.h"
 #include "virnetdevbandwidth.h"
@@ -167,4 +170,13 @@ mymain(void)
 return ret;
 }
 
-VIRT_TEST_MAIN(mymain);
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevbandwidthmock.so")
+
+#else
+
+int main(void)
+{
+return EXIT_AM_SKIP;
+}
+
+#endif /* WITH_LINUX */
-- 
1.9.3

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


[libvirt] [PATCHv2 0/2] network: bandwidth tuning in session mode revert patch

2014-11-05 Thread Erik Skultety
Erik Skultety (2):
  qemu: revert patch - bandwidth tuning in session mode
  Iface: disallow network tuning in session mode globally

 src/qemu/qemu_command.c| 11 ---
 src/qemu/qemu_driver.c |  9 -
 src/util/virnetdevbandwidth.c  |  8 
 tests/Makefile.am  | 11 ++-
 tests/virnetdevbandwidthtest.c | 14 +-
 5 files changed, 31 insertions(+), 22 deletions(-)

-- 
1.9.3

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


[libvirt] [PATCHv2 1/2] qemu: revert patch - bandwidth tuning in session mode

2014-11-05 Thread Erik Skultety
Since there was a valid note to patch 43b67f2e about the best spot to
check for bandwidth set call while having libvirt daemon run in session
mode, this patch reverts previous changes dealing with bandwith
(also reverts adding variable @cfg in qemuDomainGetNumaParameters which
 does not have any use at the moment, but getting and unreferencing
 driver's config) in qemu_driver.c and qemu_command.c. There will be
another patch in the series which introduces the fix itself.
---
 src/qemu/qemu_command.c | 11 ---
 src/qemu/qemu_driver.c  |  9 -
 2 files changed, 20 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 917639e..956bb14 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7841,17 +7841,6 @@ qemuBuildCommandLine(virConnectPtr conn,
_("CPU tuning is not available in session mode"));
 goto error;
 }
-
-virDomainNetDefPtr *nets = def->nets;
-virNetDevBandwidthPtr bandwidth = NULL;
-size_t nnets = def->nnets;
-for (i = 0; i < nnets; i++) {
-if ((bandwidth = virDomainNetGetActualBandwidth(nets[i])) != NULL) 
{
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-_("Network bandwidth tuning is not available in session 
mode"));
-goto error;
-}
-}
 }
 
 for (i = 0; i < def->ngraphics; ++i) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6acaea8..edd82c1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9383,7 +9383,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
 size_t i;
 virDomainObjPtr vm = NULL;
 virDomainDefPtr persistentDef = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
 char *nodeset = NULL;
 int ret = -1;
 virCapsPtr caps = NULL;
@@ -9402,7 +9401,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
 return -1;
 
 priv = vm->privateData;
-cfg = virQEMUDriverGetConfig(driver);
 
 if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
@@ -9476,7 +9474,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
 if (vm)
 virObjectUnlock(vm);
 virObjectUnref(caps);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -10447,12 +10444,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 if (virDomainSetInterfaceParametersEnsureACL(dom->conn, vm->def, flags) < 
0)
 goto cleanup;
 
-if (!cfg->privileged) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-   _("Network bandwidth tuning is not available in session 
mode"));
-goto cleanup;
-}
-
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
 
-- 
1.9.3

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


[libvirt] [PATCH 0/3] Fix build without NUMA

2014-11-05 Thread Michal Privoznik
Even though these patches would qualify as build breaker fixes, I
don't think they are that critical to be pushed without somebody
else's taking a look on them.

Michal Privoznik (3):
  virnuma: Add some more comments
  private.syms: Export virDomainNumatuneSpecifiedMaxNode
  qemuxml2argvmock: Mock virNumaNodesetIsAvailable

 src/libvirt_private.syms |  1 +
 src/util/virnuma.c   | 14 +++---
 tests/qemuxml2argvmock.c |  9 +
 3 files changed, 17 insertions(+), 7 deletions(-)

-- 
2.0.4

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


[libvirt] [PATCH 2/3] private.syms: Export virDomainNumatuneSpecifiedMaxNode

2014-11-05 Thread Michal Privoznik
As of 90286418 the function is introduced. However, it's missing
an entry in the libvirt_private.syms so it can't be mocked.

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 73b8167..8895ba1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -609,6 +609,7 @@ virDomainNumatuneParseXML;
 virDomainNumatunePlacementTypeFromString;
 virDomainNumatunePlacementTypeToString;
 virDomainNumatuneSet;
+virDomainNumatuneSpecifiedMaxNode;
 
 
 # conf/nwfilter_conf.h
-- 
2.0.4

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


[libvirt] [PATCH 1/3] virnuma: Add some more comments

2014-11-05 Thread Michal Privoznik
Especially add comments to mark ifdef, else and endif sections.

Signed-off-by: Michal Privoznik 
---
 src/util/virnuma.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 89435de..ee1c4af 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -74,7 +74,7 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus,
 virCommandFree(cmd);
 return output;
 }
-#else
+#else /* !HAVE_NUMAD */
 char *
 virNumaGetAutoPlacementAdvice(unsigned short vcpus ATTRIBUTE_UNUSED,
   unsigned long long balloon ATTRIBUTE_UNUSED)
@@ -83,7 +83,7 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus 
ATTRIBUTE_UNUSED,
_("numad is not available on this host"));
 return NULL;
 }
-#endif
+#endif /* !HAVE_NUMAD */
 
 #if WITH_NUMACTL
 int
@@ -339,7 +339,8 @@ virNumaGetNodeCPUs(int node,
 # undef MASK_CPU_ISSET
 # undef n_bits
 
-#else
+#else /* !WITH_NUMACTL */
+
 int
 virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
  virBitmapPtr nodemask ATTRIBUTE_UNUSED)
@@ -404,8 +405,7 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED,
_("NUMA isn't available on this host"));
 return -1;
 }
-#endif
-
+#endif /* !WITH_NUMACTL */
 
 /**
  * virNumaGetMaxCPUs:
@@ -494,8 +494,8 @@ virNumaGetDistances(int node,
 return ret;
 }
 
+#else /* !(WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET) */
 
-#else
 bool
 virNumaNodeIsAvailable(int node)
 {
@@ -519,7 +519,7 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED,
 VIR_DEBUG("NUMA distance information isn't available on this host");
 return 0;
 }
-#endif
+#endif /* !(WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET) */
 
 
 /* currently all the huge page stuff below is linux only */
-- 
2.0.4

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


[libvirt] [PATCH 3/3] qemuxml2argvmock: Mock virNumaNodesetIsAvailable

2014-11-05 Thread Michal Privoznik
And yet again one fix of 90286418. The problem is, if libvirt is
built with NUMA disabled (--without-numad --without-numactl), the
testsuite doesn't count on that and some tests fail. This is
because the virNumaNodesetIsAvailable() function is taken from
another section of virnuma.c which contains functions stubs for
case NUMA is disabled at build time. And the function basically
yields an error. However, we want our testsuite to be
deterministic and hence we ought to mock the function.

Signed-off-by: Michal Privoznik 
---
 tests/qemuxml2argvmock.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 7218747..7b8bdd6 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -39,3 +39,12 @@ virNumaGetMaxNode(void)
 
return maxnodesNum;
 }
+
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune);
+int maxHostNode = virNumaGetMaxNode();
+
+return maxGuestNode <= maxHostNode;
+}
-- 
2.0.4

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


[libvirt] [PATCH] CVE-2014-7823: dumpxml: security hole with migratable flag

2014-11-05 Thread Eric Blake
Commit 28f8dfd (v1.0.0) introduced a security hole: in at least
the qemu implementation of virDomainGetXMLDesc, the use of the
flag VIR_DOMAIN_XML_MIGRATABLE (which is usable from a read-only
connection) triggers the implicit use of VIR_DOMAIN_XML_SECURE
prior to calling qemuDomainFormatXML.  However, the use of
VIR_DOMAIN_XML_SECURE is supposed to be restricted to read-write
clients only.  This patch treats the migratable flag as requiring
the same permissions, rather than analyzing what might break if
migratable xml no longer includes secret information.

Fortunately, the information leak is low-risk: all that is gated
by the VIR_DOMAIN_XML_SECURE flag is the VNC connection password;
but VNC passwords are already weak (FIPS forbids their use, and
on a non-FIPS machine, anyone stupid enough to trust a max-8-byte
password sent in plaintext over the network deserves what they
get).  SPICE offers better security than VNC, and all other
secrets are properly protected by use of virSecret associations
rather than direct output in domain XML.

* src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_GET_XML_DESC):
Tighten rules on use of migratable flag.
* src/libvirt-domain.c (virDomainGetXMLDesc): Likewise.

Signed-off-by: Eric Blake 
---

The libvirt-security list agreed that this did not need an embargo
because it is low-risk; but I'm on the road this week, so while
this patch for master can go in now, I won't complete the backport
to all the affected stable branches (everything since v1.0.0) or
do the Libvirt Security Notice writeup until Monday.

 src/libvirt-domain.c | 3 ++-
 src/remote/remote_protocol.x | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7dc3146..2b0defc 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -2607,7 +2607,8 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int 
flags)
 virCheckDomainReturn(domain, NULL);
 conn = domain->conn;

-if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) {
+if ((conn->flags & VIR_CONNECT_RO) &&
+(flags & (VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE))) {
 virReportError(VIR_ERR_OPERATION_DENIED, "%s",
_("virDomainGetXMLDesc with secure flag"));
 goto error;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index db12cda..ebf4530 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3255,6 +3255,7 @@ enum remote_procedure {
  * @generate: both
  * @acl: domain:read
  * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE
+ * @acl: domain:read_secure:VIR_DOMAIN_XML_MIGRATABLE
  */
 REMOTE_PROC_DOMAIN_GET_XML_DESC = 14,

-- 
1.9.3

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


Re: [libvirt] [PATCH] UpdateDevice: Allow startupPolicy update

2014-11-05 Thread Peter Krempa
Add qemu: prefix to subject and mention that it's only with the _CONFIG
flag.

On 11/05/14 14:11, Michal Privoznik wrote:
> Users might want to update startupPolicy via the
> virDomainUpdateDeviceFlags API too. This patch
> implements the feature on config layer.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  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 6acaea8..6fc15c0 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7437,6 +7437,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
>  if (disk->src->format)
>  orig->src->format = disk->src->format;
>  disk->src->path = NULL;
> +orig->startupPolicy = disk->startupPolicy;

Looks on par with what we are doing now. I wanted to comment that if you
don't specify a startup policy you might want to keep the existing one,
but the code does not use this approach, thus ...

>  break;
>  
>  case VIR_DOMAIN_DEVICE_NET:
> 

ACK with the subject tweaked





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

Re: [libvirt] [PATCH 1/3] Memory: Use consistent type for all memory elements.

2014-11-05 Thread Michal Privoznik

On 05.11.2014 11:58, Prerna Saxena wrote:



From 4b3e336ea045759758b04440d75802e990506e2b Mon Sep 17 00:00:00 2001

From: Prerna Saxena 
Date: Fri, 31 Oct 2014 16:07:21 +0530

Domain memory elements such as max_balloon and cur_balloon are
implemented as 'unsigned long long', whereas the 'memory' element
in NUMA cells is implemented as 'unsigned int'.

Use the same data type (unsigned long long) for 'memory' element
in NUMA cells.

Signed-off-by: Prerna Saxena 
---
  src/conf/cpu_conf.c | 4 ++--
  src/conf/cpu_conf.h | 2 +-
  src/qemu/qemu_command.c | 6 +++---
  3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 9b7fbb0..5475c07 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -496,7 +496,7 @@ virCPUDefParseXML(xmlNodePtr node,
  goto error;
  }

-ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem);
+ret = virStrToLong_ull(memory, NULL, 10, 
&def->cells[cur_cell].mem);
  if (ret == -1) {
  virReportError(VIR_ERR_XML_ERROR, "%s",
 _("Invalid 'memory' attribute in NUMA cell"));
@@ -702,7 +702,7 @@ virCPUDefFormatBuf(virBufferPtr buf,
  virBufferAddLit(buf, "cells[i].cpustr);
-virBufferAsprintf(buf, " memory='%d'", def->cells[i].mem);
+virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem);
  if (memAccess)
  virBufferAsprintf(buf, " memAccess='%s'",
virMemAccessTypeToString(memAccess));
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index d45210b..5bcf101 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -105,7 +105,7 @@ typedef virCellDef *virCellDefPtr;
  struct _virCellDef {
  virBitmapPtr cpumask; /* CPUs that are part of this node */
  char *cpustr; /* CPUs stored in string form for dumpxml */
-unsigned int mem; /* Node memory in kB */
+unsigned long long mem; /* Node memory in kB */
  virMemAccess memAccess;
  };

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 917639e..13b54dd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6693,7 +6693,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
  }

  for (i = 0; i < def->cpu->ncells; i++) {
-int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
+unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
  def->cpu->cells[i].mem = cellmem * 1024;
  virMemAccess memAccess = def->cpu->cells[i].memAccess;

@@ -6799,7 +6799,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
  virBufferAddLit(&buf, "memory-backend-ram");
  }

-virBufferAsprintf(&buf, ",size=%dM,id=ram-node%zu", cellmem, i);
+virBufferAsprintf(&buf, ",size=%lluM,id=ram-node%zu", cellmem, i);

  if (virDomainNumatuneMaybeFormatNodeset(def->numatune, nodeset,
  &nodemask, i) < 0)
@@ -6849,7 +6849,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
  virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
  virBufferAsprintf(&buf, ",memdev=ram-node%zu", i);
  } else {
-virBufferAsprintf(&buf, ",mem=%d", cellmem);
+virBufferAsprintf(&buf, ",mem=%llu", cellmem);
  }

  virCommandAddArgBuffer(cmd, &buf);



ACKed & pushed as this doesn't depend on the rest of the patches.

Michal

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


Re: [libvirt] [PATCH 0/3] Libvirt memory & NUMA fixes

2014-11-05 Thread Michal Privoznik

On 05.11.2014 11:56, Prerna Saxena wrote:

This patch set addresses a bunch of memory & NUMA fixes.


Series Description:
===
Patch 1/3 : Use consistent data type to represent memory elements in various 
XML attributes. This ensures all memory elements are always represented as 
'unsigned long long'.
Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of a 
NUMA cell. This enables users to easily describe how much memory needs to be 
allocated to each NUMA cell for a guest domain.
Patch 3/3 : This augments test cases to add the 'unit' tag.

Regards,

The 2/3 and 3/3 should be merged together so that 'make check' won't 
fail after 2/3. Otherwise looking good. I've merged 1/3 already.


Michal

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


Re: [libvirt] [PATCH 2/3] Numa : Allow specification of 'units' for numa nodes.

2014-11-05 Thread Michal Privoznik

On 05.11.2014 12:06, Prerna Saxena wrote:



From 7fe8487c5e8d24086637d2157bad25322b3654f7 Mon Sep 17 00:00:00 2001

From: Prerna Saxena 
Date: Mon, 3 Nov 2014 07:53:59 +0530


CPU numa topology implicitly allows memory specification in 'KiB'.

Enabling this to accept the 'unit' in which memory needs to be specified.
This now allows users to specify memory in units of choice, and
lists the same in 'KiB' -- just like other 'memory' elements in XML.

 
   
   
 

I wanted to use virDomainParseScaledValue(), but that function
implicitly assumes an XML layout where 'memory' is an _element_ of type
'scaledInteger', with 'unit' as its attribute.
A NUMA cell has XML specification where 'memory' is an _attribute_ of
element 'cell'. Since changing XML layout is not an option, I have borrowed 
code from the same.


Yeah, I did the same in virDomainHugepagesParseXML. Maybe you can rename 
it to something more generic and reuse it here?




Looking forward to suggestions on how this can best be done.

Signed-off-by: Prerna Saxena 
---
  docs/schemas/domaincommon.rng |  5 +
  src/conf/cpu_conf.c   | 36 ++--
  2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 20d81ae..44cabad 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4144,6 +4144,11 @@
  


+
+  
+
+  
+  


Any XML change requires docs update. I can't ACK this with lacking 
documentation, sorry.



  

  shared
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 5475c07..65b9815 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -189,6 +189,7 @@ virCPUDefParseXML(xmlNodePtr node,
  char *cpuMode;
  char *fallback = NULL;
  char *vendor_id = NULL;
+unsigned long long max;

  if (!xmlStrEqual(node->name, BAD_CAST "cpu")) {
  virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -439,10 +440,20 @@ virCPUDefParseXML(xmlNodePtr node,

  def->ncells = n;

+/* 32 vs 64 bit will differ in value of upper memory bound.
+ * On 32-bit machines, our bound is 0x * KiB. On 64-bit
+ * machines, our bound is off_t (2^63).
+ */
+if (sizeof(unsigned long) < sizeof(long long))
+max = 1024ull * ULONG_MAX;
+else
+max = LLONG_MAX;
+
  for (i = 0; i < n; i++) {
-char *cpus, *memory, *memAccessStr;
+char *cpus, *memory, *memAccessStr, *unit;
  int ret, ncpus = 0;
  unsigned int cur_cell;
+unsigned long long bytes;
  char *tmp = NULL;

  tmp = virXMLPropString(nodes[i], "id");
@@ -496,14 +507,34 @@ virCPUDefParseXML(xmlNodePtr node,
  goto error;
  }

-ret = virStrToLong_ull(memory, NULL, 10, 
&def->cells[cur_cell].mem);
+ret = virStrToLong_ull(memory, NULL, 10, &bytes);
  if (ret == -1) {
  virReportError(VIR_ERR_XML_ERROR, "%s",
 _("Invalid 'memory' attribute in NUMA cell"));
  VIR_FREE(memory);
  goto error;
  }
+
+unit = virXMLPropString(nodes[i], "unit");
+if (!unit) {
+if (VIR_STRDUP(unit, "KiB") < 0)
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Error processing 'memory' in NUMA cell"));
+}
+
+if (virScaleInteger(&bytes, unit, 1024, max) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Error scaling 'memory' in NUMA cell"));
+VIR_FREE(memory);
+VIR_FREE(unit);
+goto error;
+}
+
+def->cells[cur_cell].mem = VIR_DIV_UP(bytes, 1024);
+
+bytes = 0;
  VIR_FREE(memory);
+VIR_FREE(unit);

  memAccessStr = virXMLPropString(nodes[i], "memAccess");
  if (memAccessStr) {
@@ -703,6 +734,7 @@ virCPUDefFormatBuf(virBufferPtr buf,
  virBufferAsprintf(buf, " id='%zu'", i);
  virBufferAsprintf(buf, " cpus='%s'", def->cells[i].cpustr);
  virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem);
+virBufferAddLit(buf, " unit='KiB'");
  if (memAccess)
  virBufferAsprintf(buf, " memAccess='%s'",
virMemAccessTypeToString(memAccess));



Michal

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


Re: [libvirt] Local qemu migration

2014-11-05 Thread Marc-André Lureau
On Wed, Nov 5, 2014 at 2:54 PM, Eric Blake  wrote:

> The monitor socket location is different for qemu:///system than for
> qemu:///session (or when going between two different users'
> qemu:///session).  But things like the VNC port do start to be an issue.
>
>
Not if the port is allocated, or the unix path is namespaced. But I
understand better where the problems may come from. Still, I would imagine
that libvirt would simply fail to migrate and no bad things would happen in
those cases.

>
> > Migration fundamentally requires that the two QEMU processes involved
> have
> > completely separate filesystem namespaces, which effectively means
> separate
> > hosts (or at least separate containers which is effectively the same
> thing)
> >
> >> Except the obvious testing case, there are other use cases that could be
> >> interesting: moving a running VM from system to session libvirt, or the
> >> other way around, or to a different user.
> >
> > Migrating from session to system libvirt or vica-verca isn't something
> > that is reasonable to support either because of the different privilege
> > levels. The session QEMU will require the disk images to be owned by
> > one user account, the system QEMU will require them to be owned by a
> > different user account. We can't chown the images to satisfy both the
> > system and session QEMU at the same time.
>
> This, coupled with the fact that we don't allow connection to a remote
> qemu:///session instance (right now, the RPC code assumes that it will
> only connect to qemu:///system), means that it is not possible to do
> live migration in or out of a session instance, regardless of whether it
> is on the same machine, and regardless of whether it is between the
> session libvirtd of two different users.
>

Well, I managed to migrate from system to session with a disk less VM, it
was useful for testing.

All you have to do is to provide the unix socket path, ex:
qemu+unix:///session?socket=/run/user/1000/libvirt/libvirt-sock

Although it would be nice to make this easier to lookup eventually


>
> It might be possible to migrate to file (such as virsh save), then
> update permissions on the save file and other resources, then reload
> from that file; but it is not a live migration.
>

Yes, that's what Boxes is doing afaik. I should have stated clearly I was
talking about live migration

cheers

-- 
Marc-André Lureau
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] nwfilter: fix deadlock caused updating network device and nwfilter

2014-11-05 Thread Pavel Hrdina
Commit 6e5c79a1 tried to fix deadlock between nwfilter{Define,Undefine}
and starting of guest, but this same deadlock is also for
updating/attaching network device to domain.

The deadlock was introduced by removing global QEMU driver lock because
nwfilter was counting on this lock and ensure that all driver locks are
locked inside of nwfilter{Define,Undefine}.

This patch extends usage of virNWFilterReadLockFilterUpdates to prevent
the deadlock for all possible paths in QEMU driver. LXC and UML drivers
still have global lock.

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

Signed-off-by: Pavel Hrdina 
---

This is temporary fix for the deadlock issue as I'm planning to create global
libvirt jobs (similar to QEMU domain jobs) and use it for other drivers and
for example for nwfilters too.

 src/qemu/qemu_driver.c| 12 
 src/qemu/qemu_migration.c |  3 +++
 src/qemu/qemu_process.c   |  4 
 3 files changed, 19 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6acaea8..9e6f505 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5937,6 +5937,8 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 def = tmp;
 }
 
+virNWFilterReadLockFilterUpdates();
+
 if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
@@ -5978,6 +5980,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 virFileWrapperFdFree(wrapperFd);
 if (vm)
 virObjectUnlock(vm);
+virNWFilterUnlockFilterUpdates();
 return ret;
 }
 
@@ -7502,6 +7505,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, 
const char *xml,
 
 affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
 
+virNWFilterReadLockFilterUpdates();
+
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
 
@@ -7614,6 +7619,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, 
const char *xml,
 virObjectUnlock(vm);
 virObjectUnref(caps);
 virObjectUnref(cfg);
+virNWFilterUnlockFilterUpdates();
 return ret;
 }
 
@@ -7644,6 +7650,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_CONFIG |
   VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
 
+virNWFilterReadLockFilterUpdates();
+
 cfg = virQEMUDriverGetConfig(driver);
 
 affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
@@ -7760,6 +7768,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 virObjectUnlock(vm);
 virObjectUnref(caps);
 virObjectUnref(cfg);
+virNWFilterUnlockFilterUpdates();
 return ret;
 }
 
@@ -14510,6 +14519,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
  * and use of FORCE can cause multiple transitions.
  */
 
+virNWFilterReadLockFilterUpdates();
+
 if (!(vm = qemuDomObjFromSnapshot(snapshot)))
 return -1;
 
@@ -14831,6 +14842,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 virObjectUnlock(vm);
 virObjectUnref(caps);
 virObjectUnref(cfg);
+virNWFilterUnlockFilterUpdates();
 
 return ret;
 }
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 94a4cf6..18242ae 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2666,6 +2666,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
+virNWFilterReadLockFilterUpdates();
+
 if (!(vm = virDomainObjListAdd(driver->domains, *def,
driver->xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
@@ -2825,6 +2827,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 qemuDomainEventQueue(driver, event);
 qemuMigrationCookieFree(mig);
 virObjectUnref(caps);
+virNWFilterUnlockFilterUpdates();
 return ret;
 
  stop:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 26d4948..409a672 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3422,6 +3422,8 @@ qemuProcessReconnect(void *opaque)
 
 VIR_FREE(data);
 
+virNWFilterReadLockFilterUpdates();
+
 virObjectLock(obj);
 
 cfg = virQEMUDriverGetConfig(driver);
@@ -3573,6 +3575,7 @@ qemuProcessReconnect(void *opaque)
 
 virObjectUnref(conn);
 virObjectUnref(cfg);
+virNWFilterUnlockFilterUpdates();
 
 return;
 
@@ -3608,6 +3611,7 @@ qemuProcessReconnect(void *opaque)
 }
 virObjectUnref(conn);
 virObjectUnref(cfg);
+virNWFilterUnlockFilterUpdates();
 }
 
 static int
-- 
2.0.4

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


Re: [libvirt] Local qemu migration

2014-11-05 Thread Eric Blake
On 11/05/2014 01:28 PM, Daniel P. Berrange wrote:
> On Wed, Nov 05, 2014 at 01:16:08PM +0100, Marc-André Lureau wrote:
>> Hi
>>
>> On Wed, Nov 5, 2014 at 9:10 AM, Daniel P. Berrange 
>> wrote:
>>
>>> Nope, there's stuff that's on the filesystem that is tied to the QEMU
>>> process and that would clash when migrating as you have two copies of
>>> the same VM running at the sme time.
>>>
>>
>> Ok, there is no guarantee that the VMs won't write concurrently to the disk
>> during migration?
>>
>> Also, there are cases where VM disks/images are readonly, or image-less VMs.
> 
> No, it isn't about the disk images. The actual guest CPUs are only executing
> in one QEMU at a time, so there's not a problem wrt disk image usage. The
> issue is with various other files we use. For example the QEMU monitor is a
> UNIX domain socket at a specific path - you can't have both QEMUs have the
> same UNIX domain socket. That's not under user control so we could change
> the monitor socket path, but the problem extends to stuff that is directly
> from the guest XML. ie VNC can be told to listen on a UNIX domain socket
> path. virtio-serial devices are typically bound to UNIX domain sockets.
> Serial ports, parallel ports, certain network device backends, and so on.

The monitor socket location is different for qemu:///system than for
qemu:///session (or when going between two different users'
qemu:///session).  But things like the VNC port do start to be an issue.

> 
> Migration fundamentally requires that the two QEMU processes involved have
> completely separate filesystem namespaces, which effectively means separate
> hosts (or at least separate containers which is effectively the same thing)
> 
>> Except the obvious testing case, there are other use cases that could be
>> interesting: moving a running VM from system to session libvirt, or the
>> other way around, or to a different user.
> 
> Migrating from session to system libvirt or vica-verca isn't something
> that is reasonable to support either because of the different privilege
> levels. The session QEMU will require the disk images to be owned by
> one user account, the system QEMU will require them to be owned by a
> different user account. We can't chown the images to satisfy both the
> system and session QEMU at the same time.

This, coupled with the fact that we don't allow connection to a remote
qemu:///session instance (right now, the RPC code assumes that it will
only connect to qemu:///system), means that it is not possible to do
live migration in or out of a session instance, regardless of whether it
is on the same machine, and regardless of whether it is between the
session libvirtd of two different users.

It might be possible to migrate to file (such as virsh save), then
update permissions on the save file and other resources, then reload
from that file; but it is not a live migration.

-- 
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] remote: Fix memory leak in remoteConnectGetAllDomainStats

2014-11-05 Thread Michal Privoznik

On 05.11.2014 12:45, Peter Krempa wrote:

The remote call actually doesn't free the arguments array so we leak
memory in case a domain list is specified. As the remote domain list
array consists only of stolen pointers from the actual domain objects
it's sufficient just to free the array.

Valgrind message:
==1081452== 64 bytes in 1 blocks are definitely lost in loss record 632 of 726
==1081452==at 0x4C296D0: calloc (vg_replace_malloc.c:618)
==1081452==by 0x4EA5CB4: virAllocN (viralloc.c:191)
==1081452==by 0x505D21E: remoteConnectGetAllDomainStats 
(remote_driver.c:7785)
==1081452==by 0x50081AA: virDomainListGetStats (libvirt-domain.c:11080)
==1081452==by 0x155249: cmdDomstats (virsh-domain-monitor.c:2147)
==1081452==by 0x12FB73: vshCommandRun (virsh.c:1935)
==1081452==by 0x133FEB: main (virsh.c:3719)
---
  src/remote/remote_driver.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 65c04d9..d111e10 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7846,6 +7846,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
  VIR_FREE(elem);
  }
  virDomainStatsRecordListFree(tmpret);
+VIR_FREE(args.doms.doms_val);
  xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret,
   (char *) &ret);



ACK

Michal

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


[libvirt] [PATCH] UpdateDevice: Allow startupPolicy update

2014-11-05 Thread Michal Privoznik
Users might want to update startupPolicy via the
virDomainUpdateDeviceFlags API too. This patch
implements the feature on config layer.

Signed-off-by: Michal Privoznik 
---
 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 6acaea8..6fc15c0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7437,6 +7437,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
 if (disk->src->format)
 orig->src->format = disk->src->format;
 disk->src->path = NULL;
+orig->startupPolicy = disk->startupPolicy;
 break;
 
 case VIR_DOMAIN_DEVICE_NET:
-- 
2.0.4

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


[libvirt] [PATCH 3/5] qemu: Split qemuDomainSetTime into two functions

2014-11-05 Thread Michal Privoznik
This is pure code movement to dig out the function internals into
a separate functions as the code is to be reused later.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 86 +++---
 1 file changed, 54 insertions(+), 32 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b866ae8..903ca5d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1868,6 +1868,59 @@ static int qemuDomainSuspend(virDomainPtr dom)
 }
 
 
+/**
+ * qemuDomainSetTimeImpl:
+ *
+ * Domain must have a job set as the monitor is entered here.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+static int
+qemuDomainSetTimeImpl(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  long long seconds,
+  unsigned int nseconds,
+  bool rtcSync)
+{
+int rv, ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("cannot set time: qemu doesn't support "
+ "rtc-reset-reinjection command"));
+goto cleanup;
+}
+
+if (!qemuDomainAgentAvailable(priv, true))
+goto cleanup;
+
+qemuDomainObjEnterAgent(vm);
+rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync);
+qemuDomainObjExitAgent(vm);
+
+if (rv < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is not running"));
+goto cleanup;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv->mon);
+qemuDomainObjExitMonitor(driver, vm);
+
+if (rv < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+return ret;
+}
+
+
 static int
 qemuDomainResumeFlags(virDomainPtr dom,
   unsigned int flags)
@@ -17711,11 +17764,9 @@ qemuDomainSetTime(virDomainPtr dom,
   unsigned int flags)
 {
 virQEMUDriverPtr driver = dom->conn->privateData;
-qemuDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
 bool rtcSync = flags & VIR_DOMAIN_TIME_SYNC;
 int ret = -1;
-int rv;
 
 virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret);
 
@@ -17725,8 +17776,6 @@ qemuDomainSetTime(virDomainPtr dom,
 if (virDomainSetTimeEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-priv = vm->privateData;
-
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
@@ -17736,34 +17785,7 @@ qemuDomainSetTime(virDomainPtr dom,
 goto endjob;
 }
 
-if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-   _("cannot set time: qemu doesn't support "
- "rtc-reset-reinjection command"));
-goto endjob;
-}
-
-if (!qemuDomainAgentAvailable(priv, true))
-goto endjob;
-
-qemuDomainObjEnterAgent(vm);
-rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync);
-qemuDomainObjExitAgent(vm);
-
-if (rv < 0)
-goto endjob;
-
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
-goto endjob;
-}
-
-qemuDomainObjEnterMonitor(driver, vm);
-rv = qemuMonitorRTCResetReinjection(priv->mon);
-qemuDomainObjExitMonitor(driver, vm);
-
-if (rv < 0)
+if (qemuDomainSetTimeImpl(driver, vm, seconds, nseconds, rtcSync) < 0)
 goto endjob;
 
 ret = 0;
-- 
2.0.4

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


[libvirt] [PATCH 1/5] Introduce virDomainResumeFlags API

2014-11-05 Thread Michal Privoznik
The old DomainResume API lacks flags argument. This is
unfortunate, because there may exist some use cases
where an additional work could be done on domain
resume. However, without flags it's not possible.

Signed-off-by: Michal Privoznik 
---
 include/libvirt/libvirt-domain.h |  2 ++
 src/driver-hypervisor.h  |  5 +
 src/libvirt-domain.c | 44 
 src/libvirt_public.syms  |  5 +
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 13 +++-
 src/remote_protocol-structs  |  5 +
 7 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index b28d37d..1795dd3 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -869,6 +869,8 @@ int virDomainFree   
(virDomainPtr domain);
  */
 int virDomainSuspend(virDomainPtr domain);
 int virDomainResume (virDomainPtr domain);
+int virDomainResumeFlags(virDomainPtr domain,
+ unsigned int flags);
 int virDomainPMSuspendForDuration (virDomainPtr domain,
unsigned int target,
unsigned long long 
duration,
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index ad66629..e781475 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -116,6 +116,10 @@ typedef int
 (*virDrvDomainResume)(virDomainPtr domain);
 
 typedef int
+(*virDrvDomainResumeFlags)(virDomainPtr domain,
+   unsigned int flags);
+
+typedef int
  (*virDrvDomainPMSuspendForDuration)(virDomainPtr,
  unsigned int target,
  unsigned long long duration,
@@ -1205,6 +1209,7 @@ struct _virHypervisorDriver {
 virDrvDomainLookupByName domainLookupByName;
 virDrvDomainSuspend domainSuspend;
 virDrvDomainResume domainResume;
+virDrvDomainResumeFlags domainResumeFlags;
 virDrvDomainPMSuspendForDuration domainPMSuspendForDuration;
 virDrvDomainPMWakeup domainPMWakeup;
 virDrvDomainShutdown domainShutdown;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7dc3146..6dcb9ef 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -693,6 +693,50 @@ virDomainResume(virDomainPtr domain)
 
 
 /**
+ * virDomainResumeFlags:
+ * @domain: a domain object
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Resume a suspended domain, the process is restarted from the state where
+ * it was frozen by calling virDomainSuspend().
+ * This function may require privileged access
+ * Moreover, resume may not be supported if domain is in some
+ * special state like VIR_DOMAIN_PMSUSPENDED.
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int
+virDomainResumeFlags(virDomainPtr domain,
+ unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+conn = domain->conn;
+
+virCheckReadOnlyGoto(conn->flags, error);
+
+if (conn->driver->domainResumeFlags) {
+int ret;
+ret = conn->driver->domainResumeFlags(domain, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain->conn);
+return -1;
+}
+
+
+/**
  * virDomainPMSuspendForDuration:
  * @dom: a domain object
  * @target: a value from virNodeSuspendTarget
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 5f95802..2daff56 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -684,4 +684,9 @@ LIBVIRT_1.2.9 {
 virNodeAllocPages;
 } LIBVIRT_1.2.8;
 
+LIBVIRT_1.2.10 {
+global:
+virDomainResumeFlags;
+} LIBVIRT_1.2.9;
+
 #  define new API here using predicted next version number 
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 65c04d9..3dc1c8f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8240,6 +8240,7 @@ static virHypervisorDriver hypervisor_driver = {
 .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 
1.2.7 */
 .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */
 .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */
+.domainResumeFlags = remoteDomainResumeFlags, /* 1.2.10 */
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index db12cda..dfd816e 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -803,6 +803,11 @@ struct remote_domain_resume_args {
 remote_nonnull_domain dom;

[libvirt] [PATCH 4/5] qemuDomainResumeFlags: Introduce VIR_DOMAIN_RESUME_SYNC_TIME

2014-11-05 Thread Michal Privoznik
This flag can be used to sync the domain's time right after
domain CPUs are started. It's basically backing call of two
subsequent APIs into one:

1) virDomainResume(dom)
2) virDomainSetTime(dom, 0, 0, VIR_DOMAIN_TIME_SYNC)

Signed-off-by: Michal Privoznik 
---
 include/libvirt/libvirt-domain.h | 4 
 src/qemu/qemu_driver.c   | 7 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 1795dd3..8d763c7 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -867,6 +867,10 @@ int virDomainFree   
(virDomainPtr domain);
 /*
  * Domain suspend/resume
  */
+typedef enum {
+VIR_DOMAIN_RESUME_SYNC_TIME = 1 << 0,   /* on resume sync domain time from 
its RTC */
+} virDomainResumeFlagsValues;
+
 int virDomainSuspend(virDomainPtr domain);
 int virDomainResume (virDomainPtr domain);
 int virDomainResumeFlags(virDomainPtr domain,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 903ca5d..38926c4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1933,7 +1933,7 @@ qemuDomainResumeFlags(virDomainPtr dom,
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_RESUME_SYNC_TIME, -1);
 
 if (!(vm = qemuDomObjFromDomain(dom)))
 return -1;
@@ -1974,6 +1974,11 @@ qemuDomainResumeFlags(virDomainPtr dom,
 goto endjob;
 if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
 goto endjob;
+
+if (flags & VIR_DOMAIN_RESUME_SYNC_TIME &&
+qemuDomainSetTimeImpl(driver, vm, 0, 0, true) < 0)
+goto endjob;
+
 ret = 0;
 
  endjob:
-- 
2.0.4

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


[libvirt] [PATCH 0/5] Allow time sync on domain resume

2014-11-05 Thread Michal Privoznik
*** BLURB HERE ***

Michal Privoznik (5):
  Introduce virDomainResumeFlags API
  Implement virDomainResumeFlags in all drivers
  qemu: Split qemuDomainSetTime into two functions
  qemuDomainResumeFlags: Introduce VIR_DOMAIN_RESUME_SYNC_TIME
  virsh: Implement virDomainResumeFlags

 include/libvirt/libvirt-domain.h |   6 +++
 src/driver-hypervisor.h  |   5 ++
 src/esx/esx_driver.c |  14 +-
 src/hyperv/hyperv_driver.c   |  14 +-
 src/libvirt-domain.c |  44 
 src/libvirt_public.syms  |   5 ++
 src/libxl/libxl_driver.c |  14 +-
 src/lxc/lxc_driver.c |  15 +-
 src/openvz/openvz_driver.c   |  13 -
 src/parallels/parallels_driver.c |  11 +++-
 src/phyp/phyp_driver.c   |  12 -
 src/qemu/qemu_driver.c   | 106 ++-
 src/remote/remote_driver.c   |   1 +
 src/remote/remote_protocol.x |  13 -
 src/remote_protocol-structs  |   5 ++
 src/test/test_driver.c   |  13 -
 src/vbox/vbox_common.c   |  11 +++-
 src/vmware/vmware_driver.c   |  12 -
 src/xen/xen_driver.c |  14 +-
 src/xenapi/xenapi_driver.c   |  21 +++-
 tools/virsh-domain.c |  13 -
 tools/virsh.pod  |  10 ++--
 22 files changed, 316 insertions(+), 56 deletions(-)

-- 
2.0.4

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


[libvirt] [PATCH 2/5] Implement virDomainResumeFlags in all drivers

2014-11-05 Thread Michal Privoznik
This practically boils down to:
1) rename DomainResume implementation to DomainResumeFlags
2) make DomainResume call DomainResumeFlags(dom, 0);

Signed-off-by: Michal Privoznik 
---
 src/esx/esx_driver.c | 14 +-
 src/hyperv/hyperv_driver.c   | 14 +-
 src/libxl/libxl_driver.c | 14 --
 src/lxc/lxc_driver.c | 15 +--
 src/openvz/openvz_driver.c   | 13 -
 src/parallels/parallels_driver.c | 11 ++-
 src/phyp/phyp_driver.c   | 12 +++-
 src/qemu/qemu_driver.c   | 15 +--
 src/test/test_driver.c   | 13 -
 src/vbox/vbox_common.c   | 11 ++-
 src/vmware/vmware_driver.c   | 12 +++-
 src/xen/xen_driver.c | 14 --
 src/xenapi/xenapi_driver.c   | 21 +++--
 13 files changed, 161 insertions(+), 18 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 0770e89..aaca532 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1780,7 +1780,8 @@ esxDomainSuspend(virDomainPtr domain)
 
 
 static int
-esxDomainResume(virDomainPtr domain)
+esxDomainResumeFlags(virDomainPtr domain,
+ unsigned int flags)
 {
 int result = -1;
 esxPrivate *priv = domain->conn->privateData;
@@ -1791,6 +1792,8 @@ esxDomainResume(virDomainPtr domain)
 esxVI_TaskInfoState taskInfoState;
 char *taskInfoErrorMessage = NULL;
 
+virCheckFlags(0, -1);
+
 if (esxVI_EnsureSession(priv->primary) < 0) {
 return -1;
 }
@@ -1838,6 +1841,14 @@ esxDomainResume(virDomainPtr domain)
 
 
 static int
+esxDomainResume(virDomainPtr domain)
+{
+return esxDomainResumeFlags(domain, 0);
+}
+
+
+
+static int
 esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags)
 {
 int result = -1;
@@ -5310,6 +5321,7 @@ static virHypervisorDriver esxDriver = {
 .domainLookupByName = esxDomainLookupByName, /* 0.7.0 */
 .domainSuspend = esxDomainSuspend, /* 0.7.0 */
 .domainResume = esxDomainResume, /* 0.7.0 */
+.domainResumeFlags = esxDomainResumeFlags, /* 1.2.10 */
 .domainShutdown = esxDomainShutdown, /* 0.7.0 */
 .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.10 */
 .domainReboot = esxDomainReboot, /* 0.7.0 */
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 5ffcb85..ac8f745 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -569,12 +569,15 @@ hypervDomainSuspend(virDomainPtr domain)
 
 
 static int
-hypervDomainResume(virDomainPtr domain)
+hypervDomainResumeFlags(virDomainPtr domain,
+unsigned int flags)
 {
 int result = -1;
 hypervPrivate *priv = domain->conn->privateData;
 Msvm_ComputerSystem *computerSystem = NULL;
 
+virCheckFlags(0, -1);
+
 if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) {
 goto cleanup;
 }
@@ -598,6 +601,14 @@ hypervDomainResume(virDomainPtr domain)
 
 
 static int
+hypervDomainResume(virDomainPtr domain)
+{
+return hypervDomainResumeFlags(domain, 0);
+}
+
+
+
+static int
 hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags)
 {
 int result = -1;
@@ -1370,6 +1381,7 @@ static virHypervisorDriver hypervDriver = {
 .domainLookupByName = hypervDomainLookupByName, /* 0.9.5 */
 .domainSuspend = hypervDomainSuspend, /* 0.9.5 */
 .domainResume = hypervDomainResume, /* 0.9.5 */
+.domainResumeFlags = hypervDomainResumeFlags, /* 1.2.10 */
 .domainDestroy = hypervDomainDestroy, /* 0.9.5 */
 .domainDestroyFlags = hypervDomainDestroyFlags, /* 0.9.5 */
 .domainGetOSType = hypervDomainGetOSType, /* 0.9.5 */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d2c077c..342d0a5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -814,7 +814,8 @@ libxlDomainSuspend(virDomainPtr dom)
 
 
 static int
-libxlDomainResume(virDomainPtr dom)
+libxlDomainResumeFlags(virDomainPtr dom,
+   unsigned int flags)
 {
 libxlDriverPrivatePtr driver = dom->conn->privateData;
 libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
@@ -823,10 +824,12 @@ libxlDomainResume(virDomainPtr dom)
 virObjectEventPtr event = NULL;
 int ret = -1;
 
+virCheckFlags(0, -1);
+
 if (!(vm = libxlDomObjFromDomain(dom)))
 goto cleanup;
 
-if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0)
+if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
 if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
@@ -873,6 +876,12 @@ libxlDomainResume(virDomainPtr dom)
 }
 
 static int
+libxlDomainResume(virDomainPtr dom)
+{
+return libxlDomainResumeFlags(dom, 0);
+}
+
+static int
 libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
 {
 virDomainObjPtr vm;
@@ -4750,6 +4759,7 @@ static virHypervisorDriver libxlDriver = {
 .domainLookupByName

[libvirt] [PATCH 5/5] virsh: Implement virDomainResumeFlags

2014-11-05 Thread Michal Privoznik
This is done by introducing '--sync-time' to already existing
'resume' command. If the option is used, the newer Flags() API
is called.

Signed-off-by: Michal Privoznik 
---
 tools/virsh-domain.c | 13 -
 tools/virsh.pod  | 10 ++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index dfc3a8c..c02f0a2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5135,6 +5135,10 @@ static const vshCmdOptDef opts_resume[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_("domain name, id or uuid")
 },
+{.name = "sync-time",
+ .type = VSH_OT_BOOL,
+ .help = N_("sync domain's time from its RTC")
+},
 {.name = NULL}
 };
 
@@ -5144,11 +5148,18 @@ cmdResume(vshControl *ctl, const vshCmd *cmd)
 virDomainPtr dom;
 bool ret = true;
 const char *name;
+unsigned int flags = 0;
 
 if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
 return false;
 
-if (virDomainResume(dom) == 0) {
+if (vshCommandOptBool(cmd, "sync-time"))
+flags |= VIR_DOMAIN_RESUME_SYNC_TIME;
+
+if ((flags &&
+ virDomainResumeFlags(dom, flags) == 0) ||
+(!flags &&
+ virDomainResume(dom) == 0)) {
 vshPrint(ctl, _("Domain %s resumed\n"), name);
 } else {
 vshError(ctl, _("Failed to resume domain %s"), name);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index daddb48..001f61a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2101,11 +2101,13 @@ is only supported with container based virtualization.
 Suspend a running domain. It is kept in memory but won't be scheduled
 anymore.
 
-=item B I
+=item B I [I<--sync-time>]
 
-Moves a domain out of the suspended state.  This will allow a previously
-suspended domain to now be eligible for scheduling by the underlying
-hypervisor.
+Moves a domain out of the suspended state.  This will allow a
+previously suspended domain to now be eligible for scheduling by the
+underlying hypervisor. Moreover, if I<--sync-time> is specified the
+domain's time is synced right after the domain CPUs are started. Note,
+that this may require guest agent on some hypervisors.
 
 =item B I I [I<--duration>]
 
-- 
2.0.4

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


Re: [libvirt] Local qemu migration

2014-11-05 Thread Daniel P. Berrange
On Wed, Nov 05, 2014 at 01:16:08PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Nov 5, 2014 at 9:10 AM, Daniel P. Berrange 
> wrote:
> 
> > Nope, there's stuff that's on the filesystem that is tied to the QEMU
> > process and that would clash when migrating as you have two copies of
> > the same VM running at the sme time.
> >
> 
> Ok, there is no guarantee that the VMs won't write concurrently to the disk
> during migration?
> 
> Also, there are cases where VM disks/images are readonly, or image-less VMs.

No, it isn't about the disk images. The actual guest CPUs are only executing
in one QEMU at a time, so there's not a problem wrt disk image usage. The
issue is with various other files we use. For example the QEMU monitor is a
UNIX domain socket at a specific path - you can't have both QEMUs have the
same UNIX domain socket. That's not under user control so we could change
the monitor socket path, but the problem extends to stuff that is directly
from the guest XML. ie VNC can be told to listen on a UNIX domain socket
path. virtio-serial devices are typically bound to UNIX domain sockets.
Serial ports, parallel ports, certain network device backends, and so on.

Migration fundamentally requires that the two QEMU processes involved have
completely separate filesystem namespaces, which effectively means separate
hosts (or at least separate containers which is effectively the same thing)

> Except the obvious testing case, there are other use cases that could be
> interesting: moving a running VM from system to session libvirt, or the
> other way around, or to a different user.

Migrating from session to system libvirt or vica-verca isn't something
that is reasonable to support either because of the different privilege
levels. The session QEMU will require the disk images to be owned by
one user account, the system QEMU will require them to be owned by a
different user account. We can't chown the images to satisfy both the
system and session QEMU at the same time.

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] Local qemu migration

2014-11-05 Thread Marc-André Lureau
Hi

On Wed, Nov 5, 2014 at 9:10 AM, Daniel P. Berrange 
wrote:

> Nope, there's stuff that's on the filesystem that is tied to the QEMU
> process and that would clash when migrating as you have two copies of
> the same VM running at the sme time.
>

Ok, there is no guarantee that the VMs won't write concurrently to the disk
during migration?

Also, there are cases where VM disks/images are readonly, or image-less VMs.

Except the obvious testing case, there are other use cases that could be
interesting: moving a running VM from system to session libvirt, or the
other way around, or to a different user.

-- 
Marc-André Lureau
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] remote: Fix memory leak in remoteConnectGetAllDomainStats

2014-11-05 Thread Peter Krempa
The remote call actually doesn't free the arguments array so we leak
memory in case a domain list is specified. As the remote domain list
array consists only of stolen pointers from the actual domain objects
it's sufficient just to free the array.

Valgrind message:
==1081452== 64 bytes in 1 blocks are definitely lost in loss record 632 of 726
==1081452==at 0x4C296D0: calloc (vg_replace_malloc.c:618)
==1081452==by 0x4EA5CB4: virAllocN (viralloc.c:191)
==1081452==by 0x505D21E: remoteConnectGetAllDomainStats 
(remote_driver.c:7785)
==1081452==by 0x50081AA: virDomainListGetStats (libvirt-domain.c:11080)
==1081452==by 0x155249: cmdDomstats (virsh-domain-monitor.c:2147)
==1081452==by 0x12FB73: vshCommandRun (virsh.c:1935)
==1081452==by 0x133FEB: main (virsh.c:3719)
---
 src/remote/remote_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 65c04d9..d111e10 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7846,6 +7846,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
 VIR_FREE(elem);
 }
 virDomainStatsRecordListFree(tmpret);
+VIR_FREE(args.doms.doms_val);
 xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret,
  (char *) &ret);

-- 
2.1.0

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


[libvirt] [PATCH 2/3] Numa : Allow specification of 'units' for numa nodes.

2014-11-05 Thread Prerna Saxena

>From 7fe8487c5e8d24086637d2157bad25322b3654f7 Mon Sep 17 00:00:00 2001
From: Prerna Saxena 
Date: Mon, 3 Nov 2014 07:53:59 +0530


CPU numa topology implicitly allows memory specification in 'KiB'.

Enabling this to accept the 'unit' in which memory needs to be specified.
This now allows users to specify memory in units of choice, and
lists the same in 'KiB' -- just like other 'memory' elements in XML.


  
  


I wanted to use virDomainParseScaledValue(), but that function
implicitly assumes an XML layout where 'memory' is an _element_ of type
'scaledInteger', with 'unit' as its attribute.
A NUMA cell has XML specification where 'memory' is an _attribute_ of
element 'cell'. Since changing XML layout is not an option, I have borrowed 
code from the same.

Looking forward to suggestions on how this can best be done.

Signed-off-by: Prerna Saxena 
---
 docs/schemas/domaincommon.rng |  5 +
 src/conf/cpu_conf.c   | 36 ++--
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 20d81ae..44cabad 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4144,6 +4144,11 @@
 
   
   
+
+  
+
+  
+  
 
   
 shared
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 5475c07..65b9815 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -189,6 +189,7 @@ virCPUDefParseXML(xmlNodePtr node,
 char *cpuMode;
 char *fallback = NULL;
 char *vendor_id = NULL;
+unsigned long long max;
 
 if (!xmlStrEqual(node->name, BAD_CAST "cpu")) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -439,10 +440,20 @@ virCPUDefParseXML(xmlNodePtr node,
 
 def->ncells = n;
 
+/* 32 vs 64 bit will differ in value of upper memory bound.
+ * On 32-bit machines, our bound is 0x * KiB. On 64-bit
+ * machines, our bound is off_t (2^63).
+ */
+if (sizeof(unsigned long) < sizeof(long long))
+max = 1024ull * ULONG_MAX;
+else
+max = LLONG_MAX;
+
 for (i = 0; i < n; i++) {
-char *cpus, *memory, *memAccessStr;
+char *cpus, *memory, *memAccessStr, *unit;
 int ret, ncpus = 0;
 unsigned int cur_cell;
+unsigned long long bytes;
 char *tmp = NULL;
 
 tmp = virXMLPropString(nodes[i], "id");
@@ -496,14 +507,34 @@ virCPUDefParseXML(xmlNodePtr node,
 goto error;
 }
 
-ret = virStrToLong_ull(memory, NULL, 10, 
&def->cells[cur_cell].mem);
+ret = virStrToLong_ull(memory, NULL, 10, &bytes);
 if (ret == -1) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Invalid 'memory' attribute in NUMA cell"));
 VIR_FREE(memory);
 goto error;
 }
+
+unit = virXMLPropString(nodes[i], "unit");
+if (!unit) {
+if (VIR_STRDUP(unit, "KiB") < 0)
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Error processing 'memory' in NUMA cell"));
+}
+
+if (virScaleInteger(&bytes, unit, 1024, max) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Error scaling 'memory' in NUMA cell"));
+VIR_FREE(memory);
+VIR_FREE(unit);
+goto error;
+}
+
+def->cells[cur_cell].mem = VIR_DIV_UP(bytes, 1024);
+
+bytes = 0;
 VIR_FREE(memory);
+VIR_FREE(unit);
 
 memAccessStr = virXMLPropString(nodes[i], "memAccess");
 if (memAccessStr) {
@@ -703,6 +734,7 @@ virCPUDefFormatBuf(virBufferPtr buf,
 virBufferAsprintf(buf, " id='%zu'", i);
 virBufferAsprintf(buf, " cpus='%s'", def->cells[i].cpustr);
 virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem);
+virBufferAddLit(buf, " unit='KiB'");
 if (memAccess)
 virBufferAsprintf(buf, " memAccess='%s'",
   virMemAccessTypeToString(memAccess));
-- 
1.9.3

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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


[libvirt] [PATCH 3/3]Test:Augment test cases to correctly model NUMA specification.

2014-11-05 Thread Prerna Saxena

>From 2fe0e329e7224b7cd29e1252e4b4e70d9195ab2b Mon Sep 17 00:00:00 2001
From: Prerna Saxena 
Date: Mon, 3 Nov 2014 15:16:12 +0530

 This adds the tag 'unit="KiB"' for memory attribute in NUMA cells.

Signed-off-by: Prerna Saxena 
---
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml| 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml   | 8 
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml  | 8 
 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml  | 8 
 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml  | 2 +-
 .../qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml  | 4 ++--
 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml  | 6 +++---
 .../qemuxml2argv-numatune-memnodes-problematic.xml| 4 ++--
 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml | 4 ++--
 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml | 4 ++--
 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml  | 2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml  | 6 +++---
 18 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
index 474a238..bdffcd1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
@@ -11,8 +11,8 @@
   
 
 
-  
-  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml
index cf7c040..c638ffa 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml
@@ -11,8 +11,8 @@
   
 
 
-  
-  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml
index 0543f7f..20120e9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml
@@ -11,8 +11,8 @@
   
 
 
-  
-  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml
index 0a5f9fc..a90e7a2 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml
@@ -11,8 +11,8 @@
   
 
 
-  
-  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
index fa3070d..ea2dc81 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
@@ -11,8 +11,8 @@
   
 
 
-  
-  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml
index 5ad0695..b67df2f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml
@@ -20,10 +20,10 @@
   
   
 
-  
-  
-  
-  
+  
+  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
index 3df870b..6afa6ef 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.xml
@@ -15,8 +15,8 @@
   
   
 
-  
-  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml
index 35aa2cf..21f4985 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.xml
@@ -15,8 +15,8 @@
   
   
 
-  
-  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml
index a3ed29b..eb18f24 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml
@@ -20,10 +20,10 @@
   
   
 
-  
-  
-  
-  
+  
+  
+  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hug

[libvirt] [PATCH 1/3] Memory: Use consistent type for all memory elements.

2014-11-05 Thread Prerna Saxena

>From 4b3e336ea045759758b04440d75802e990506e2b Mon Sep 17 00:00:00 2001
From: Prerna Saxena 
Date: Fri, 31 Oct 2014 16:07:21 +0530

Domain memory elements such as max_balloon and cur_balloon are
implemented as 'unsigned long long', whereas the 'memory' element
in NUMA cells is implemented as 'unsigned int'.

Use the same data type (unsigned long long) for 'memory' element
in NUMA cells.

Signed-off-by: Prerna Saxena 
---
 src/conf/cpu_conf.c | 4 ++--
 src/conf/cpu_conf.h | 2 +-
 src/qemu/qemu_command.c | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 9b7fbb0..5475c07 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -496,7 +496,7 @@ virCPUDefParseXML(xmlNodePtr node,
 goto error;
 }
 
-ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem);
+ret = virStrToLong_ull(memory, NULL, 10, 
&def->cells[cur_cell].mem);
 if (ret == -1) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Invalid 'memory' attribute in NUMA cell"));
@@ -702,7 +702,7 @@ virCPUDefFormatBuf(virBufferPtr buf,
 virBufferAddLit(buf, "cells[i].cpustr);
-virBufferAsprintf(buf, " memory='%d'", def->cells[i].mem);
+virBufferAsprintf(buf, " memory='%llu'", def->cells[i].mem);
 if (memAccess)
 virBufferAsprintf(buf, " memAccess='%s'",
   virMemAccessTypeToString(memAccess));
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index d45210b..5bcf101 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -105,7 +105,7 @@ typedef virCellDef *virCellDefPtr;
 struct _virCellDef {
 virBitmapPtr cpumask; /* CPUs that are part of this node */
 char *cpustr; /* CPUs stored in string form for dumpxml */
-unsigned int mem; /* Node memory in kB */
+unsigned long long mem; /* Node memory in kB */
 virMemAccess memAccess;
 };
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 917639e..13b54dd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6693,7 +6693,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 }
 
 for (i = 0; i < def->cpu->ncells; i++) {
-int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
+unsigned long long cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024);
 def->cpu->cells[i].mem = cellmem * 1024;
 virMemAccess memAccess = def->cpu->cells[i].memAccess;
 
@@ -6799,7 +6799,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 virBufferAddLit(&buf, "memory-backend-ram");
 }
 
-virBufferAsprintf(&buf, ",size=%dM,id=ram-node%zu", cellmem, i);
+virBufferAsprintf(&buf, ",size=%lluM,id=ram-node%zu", cellmem, i);
 
 if (virDomainNumatuneMaybeFormatNodeset(def->numatune, nodeset,
 &nodemask, i) < 0)
@@ -6849,7 +6849,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
 virBufferAsprintf(&buf, ",memdev=ram-node%zu", i);
 } else {
-virBufferAsprintf(&buf, ",mem=%d", cellmem);
+virBufferAsprintf(&buf, ",mem=%llu", cellmem);
 }
 
 virCommandAddArgBuffer(cmd, &buf);
-- 
1.9.3


-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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


[libvirt] [PATCH 0/3] Libvirt memory & NUMA fixes

2014-11-05 Thread Prerna Saxena
This patch set addresses a bunch of memory & NUMA fixes.


Series Description:
===
Patch 1/3 : Use consistent data type to represent memory elements in various 
XML attributes. This ensures all memory elements are always represented as 
'unsigned long long'.
Patch 2/3 : This adds a 'unit' attribute alongwith the 'memory' attribute of a 
NUMA cell. This enables users to easily describe how much memory needs to be 
allocated to each NUMA cell for a guest domain.
Patch 3/3 : This augments test cases to add the 'unit' tag.

Regards,

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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


Re: [libvirt] [sandbox PATCH 0/2] v1.1 bugfix: support dhcp network interfaces

2014-11-05 Thread Michal Privoznik

On 31.10.2014 19:02, Gene Czarcinski wrote:

Resubmitted to add "sandbox PATCH" to Subject

v1.1 adds some documentation changes.

Support for a network such as -N dhcp,source=default was not working
in that dhclient was not being started.  Although I am not sure what
the real problem is, the solution is to use g_spawn_sync() instead of
g_spawn_async() to start /sbin/dhclient.

The second patch addes "-v" to the dhclient arguments to improve debugging
info.  The dhclient into will be in /var/log/messages the Secure Contrainer
host system and not in the container itself.

Gene Czarcinski (2):
   v1.1 for dhclient use g_spawn_sync()
   v1.1 add -v to dhclient parameter arguments

  libvirt-sandbox/libvirt-sandbox-init-common.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)



Even though this is not rebased as I asked (I still get conflicts when 
trying to apply the patches), I've resolved the conflicts, ACKed and pushed.


Michal

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


Re: [libvirt] [libvirt-sandbox][PATCH 0/4] Couple of fixes to compile again

2014-11-05 Thread Martin Kletzander

On Tue, Nov 04, 2014 at 03:40:47PM +0100, Michal Privoznik wrote:

It's been a while that I tried to build libvirt-sandbox. And
guess what, it doesn't compile cleanly so here are some patches
to fix the issues I met.

Michal Privoznik (4):
 virt-selinux.m4: Define SELINUX variables
 Makefile: link SELINUX into libvirt-sandbox-1.0.so
 m4: sync macros with libvirt
 libvirt-sandbox-config.c: Fix comment

libvirt-sandbox/Makefile.am  |   4 +-
libvirt-sandbox/libvirt-sandbox-config.c |   2 +-
m4/manywarnings.m4   | 217 ---
m4/virt-compile-warnings.m4  | 216 ++
m4/virt-selinux.m4   |   6 +-
m4/warnings.m4   |  82 +---
6 files changed, 369 insertions(+), 158 deletions(-)



ACK series except 3/4 (read the response to that particular one).

Martin


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

Re: [libvirt] [libvirt-sandbox][PATCH 3/4] m4: sync macros with libvirt

2014-11-05 Thread Martin Kletzander

On Tue, Nov 04, 2014 at 03:40:50PM +0100, Michal Privoznik wrote:

The macros under the m4 directory are outdated a bit. When trying
to compile with newer gcc I see some errors:

make[4]: Entering directory 
'/home/zippy/work/libvirt/libvirt-sanbox.git/libvirt-sandbox'
 CC   libvirt_sandbox_1_0_la-libvirt-sandbox-main.lo
gcc: warning: switch '-Wmudflap' is no longer supported

Signed-off-by: Michal Privoznik 
---
m4/manywarnings.m4  | 217 
m4/virt-compile-warnings.m4 | 216 +++
m4/warnings.m4  |  82 +
3 files changed, 361 insertions(+), 154 deletions(-)


[...]

diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index 894ab59..532a777 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -2,7 +2,7 @@ dnl
dnl Enable all known GCC compiler warnings, except for those
dnl we can't yet cope with
dnl
-AC_DEFUN([LIBVIRT_SANDBOX_COMPILE_WARNINGS],[
+AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[


This function changed its name due to the overwrite and ./configure
now yells an error, but it doesn't fail, so it's easy to miss.

apart from this particular file, which is specific for libvirt-related
projects, it's ok to sync the rest with gnulib, as it's being done on
regular (or rather irregular) basis.

So partial ACK for m4/manywarnings.m4 and m4/warnings.m4 only.

Martin


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

Re: [libvirt] [PATCH] Require at least one console for LXC domain

2014-11-05 Thread Chen, Hanxiao


> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Martin Kletzander
> Sent: Wednesday, November 05, 2014 4:10 PM
> To: Ján Tomko
> Cc: libvir-list@redhat.com
[snip]
> >
> >It's been broken long enough to make me think nobody cares about this
> >functionality, but I could make it work if you think it's worthwhile.
> >
> 
> Not worth while, I agree with you.  It can be done later on if someone
> really wants it.
> 
> ACK,
> 

Agree.
But we could do some improvements such as 
adding a console for containers automatically
if we do not provided one in XML.

Thanks
- Chen

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

Re: [libvirt] [PATCH] Require at least one console for LXC domain

2014-11-05 Thread Martin Kletzander

On Tue, Nov 04, 2014 at 03:21:53PM +0100, Ján Tomko wrote:

On 10/31/2014 11:53 AM, Martin Kletzander wrote:

On Fri, Oct 31, 2014 at 10:22:25AM +0100, Ján Tomko wrote:

A domain without a console quietly dies soon after start,
because we try to set /dev/null as a controlling TTY
2014-10-30 15:10:59.705+: 1: error : lxcContainerSetupFDs:283 :
ioctl(TIOCSTTY) failed: Inappropriate ioctl for device



s/TIOCSTTY/TIOCSCTTY/ would make it more greppable (is that even a
word?) and it's now true since you pushed your trivial fix for that ;)



I have amended the commit message.


Report an error early instead of trying to start it.

https://bugzilla.redhat.com/show_bug.cgi?id=1155410
---
src/lxc/lxc_container.c | 6 --
src/lxc/lxc_process.c   | 6 ++
2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index f02b959..8aba3ba 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2093,8 +2093,10 @@ static int lxcContainerChild(void *data)
if (virAsprintf(&ttyPath, "%s/%s.devpts/%s",
LXC_STATE_DIR, vmDef->name, tty) < 0)
goto cleanup;
-} else if (VIR_STRDUP(ttyPath, "/dev/null") < 0) {
-goto cleanup;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("At least one tty is required"));
+goto cleanup;
}


How about we don't attach any tty and use the ioctl(TIOCSTTY) to steal
controlling terminal (or call open() with O_NOCTTY, or just don't call
ioctl() at all, I don't know what we normally use)?  Then there would
be no controlling terminal, but there would still be a session leader,
wouldn't it?



I don't know if we want to run a container without a controlling terminal.
Looking at git history, back when we only supported one console and none was
specified, it seems we quietly opened one anyway and didn't report it in the 
XML.

Alternatively, we could auto-add a console in the XML.


Anyway, this version looks OK too since it doesn't break any use case
(there is nothing to break now).  ACK after release if you argue why
not to use the idea above.


It's been broken long enough to make me think nobody cares about this
functionality, but I could make it work if you think it's worthwhile.



Not worth while, I agree with you.  It can be done later on if someone
really wants it.

ACK,

Martin


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

Re: [libvirt] Local qemu migration

2014-11-05 Thread Daniel P. Berrange
On Tue, Nov 04, 2014 at 10:49:44AM -0500, Cole Robinson wrote:
> On 11/04/2014 10:43 AM, Marc-André Lureau wrote:
> >Hi,
> >
> >Attempting to migration from session to system qemu fails because of the
> >following checks in  qemuMigrationCookieXMLParse():
> >
> >if (STREQ(mig->remoteHostname, mig->localHostname)) {
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> >_("Attempt to migrate guest to the same host %s"),
> >mig->remoteHostname);
> > goto error;
> > }
> >
> > if (memcmp(mig->remoteHostuuid, mig->localHostuuid, VIR_UUID_BUFLEN) == 
> > 0) {
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> >_("Attempt to migrate guest to the same host %s"),
> >tmp);
> > goto error;
> >
> >Is there a technical limitation for this error? If not, could it be overriden
> >with an additional flag?
> 
> Trying to prevent migrating to the same libvirtd instance which would
> historically deadlock in libvirt's qemu driver. But migrating session ->
> system shouldn't suffer from that problem, so maybe it should be smarter.
> 
> Maybe with the more fine grained locking these days we can actually make
> localhost migration work? Would be useful for testing

Nope, there's stuff that's on the filesystem that is tied to the QEMU
process and that would clash when migrating as you have two copies of
the same VM running at the sme time.

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] qemu: don't setup cpuset.mems if memory mode in numatune is 'preferred'

2014-11-05 Thread Martin Kletzander

On Wed, Nov 05, 2014 at 12:00:14PM +0800, Wang Rui wrote:

On 2014/11/4 22:04, Martin Kletzander wrote:

On Tue, Nov 04, 2014 at 09:22:22PM +0800, Wang Rui wrote:

If the memory mode is specified as preferred, we get the following error when
starting domain.

error: Unable to write to '$my_cgroup_path/cpuset.mems': Device or resource busy

XML is configured with numatune as follows:
 
   
 

If memory mode is 'preferred', cpuset.mems in cgroup shouldn't be set to
'nodeset'. I find that maybe commit 1a7be8c600905aa07ac2d78293336ba8523ad48e
changes the former logic of checking mode in virDomainNumatuneGetNodeset.

Signed-off-by: Wang Rui 
---
src/qemu/qemu_cgroup.c | 5 +
1 file changed, 5 insertions(+)



Thanks for catching that, it definitely is a problem, but I think it
is cause by commit 93e82727ec11d471d2ef3a18835e1fdfe062cef1.

It should be also fixed in virLXCCgroupSetupCpusetTune() for LXC.


OK. I'll try to fix it for LXC in another patch.



Yeah, that can be a follow-up, it'll look the same, anyway.


diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index b5bdb36..8685d6f 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -618,6 +618,11 @@ qemuSetupCpusetMems(virDomainObjPtr vm,
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
return 0;

+if (virDomainNumatuneGetMode(vm->def->numatune, -1) !=
+VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+return 0;
+}
+


One question, is it problem only for 'preferred' or 'interleaved' as
well?  Because if it's only problem for 'preferred', then the check is
wrong.  If it's problem for 'interleaved' as well, then the commit
message is wrong.


'interleave' with a single node(such as nodeset='0') will cause the same error.
But 'interleave' mode should not live with a single node. So maybe there's
another bugfix to check 'interleave' with single node.



Well, I'd be OK with just changing the commit message to mention that.
This fix is still a valid one and will fix both issues, won't it?


If configured with 'interleave' and multiple nodes(such as nodeset='0-1'),
VM can be started successfully. And cpuset.mems is set to the same nodeset.
So I'll revise my patch.

I'll send patches V2. Conclusion:

1/3 : add check for 'interleave' mode with single numa node
2/3 : fix this problem in qemu
3/3 : fix this problem in lxc

Is it OK?


Anyway, after either one is fixed, I can push this.

Thank you,
Martin




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