date:20141030

Re: [libvirt] [PATCH 1/2] virnetdev: stub virNetDev{Add, Del}Multi on FreeBSD

2014-10-30 Thread Roman Bogorodskiy

  John Ferlan wrote:

 On 10/29/2014 02:20 PM, Roman Bogorodskiy wrote:
  Currently, build fails on FreeBSD because its struct ifreq does not
  have ifr_hwaddr member. In order to fix that, check if this member
  is present, otherwise fall back to the stub version of the
  virNetDev{Add,Del}Multi functions.
  ---
   configure.ac | 3 ++-
   src/util/virnetdev.c | 6 --
   2 files changed, 6 insertions(+), 3 deletions(-)
  
 
 Looks reasonable to me and since it fixes the build on FreeBSD...
 
 ACK

Pushed, thanks!

Roman Bogorodskiy

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

Re: [libvirt] [PATCH 2/2] virutil: fix virGetSCSIHostNumber stub return type

2014-10-30 Thread Roman Bogorodskiy

  John Ferlan wrote:

 On 10/29/2014 02:20 PM, Roman Bogorodskiy wrote:
  The virGetSCSIHostNumber function return type is int, however
  its stubbed version returns NULL. That results in a build fail
  on systems that uses the stubbed version. Fix by using a proper
  return type.
  ---
   src/util/virutil.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
 
 ACK - (me who once again gets bit by cut-n-paste of functions)

Pushed, thanks!

Roman Bogorodskiy

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

Re: [libvirt] [PATCH] Fix memory leak in cmdNetworkDHCPLeases

2014-10-30 Thread Peter Krempa

In subject: I've added virsh: designator to make clear which part of
the code the patch touches.

On 10/30/14 03:35, Luyao Huang wrote:
 After use cidr_format in function virAsprintf and vshPrintExtra, need free
 cidr_format.

And clarified this sentence a bit.

 
 Fix the following memory leak from valgrind, like:
  18 bytes in 1 blocks are definitely lost in loss record 41 of 192
 at 0x4C29BBD: malloc (in 
 /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
 by 0x85CE36F: __vasprintf_chk (vasprintf_chk.c:80)
 by 0x4EE52D5: UnknownInlinedFun (stdio2.h:210)
 by 0x4EE52D5: virVasprintfInternal (virstring.c:459)
 by 0x4EE53CA: virAsprintfInternal (virstring.c:480)
 by 0x14FE96: cmdNetworkDHCPLeases (virsh-network.c:1378)
 by 0x13006B: vshCommandRun (virsh.c:1915)
 by 0x12A9E1: main (virsh.c:3699)
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  tools/virsh-network.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/tools/virsh-network.c b/tools/virsh-network.c
 index 90392d3..8ff6fd8 100644
 --- a/tools/virsh-network.c
 +++ b/tools/virsh-network.c
 @@ -1381,6 +1381,8 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
expirytime, EMPTYSTR(lease-mac),
EMPTYSTR(typestr), cidr_format,
EMPTYSTR(lease-hostname), EMPTYSTR(lease-clientid));
 +
 + VIR_FREE(cidr_format);
  }
  
  ret = true;
 

ACK, thanks for taking your time and fixing the issue.

I'll push the patch shortly.

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] Fix memory leak in cmdNetworkDHCPLeases

2014-10-30 Thread Peter Krempa

On 10/30/14 03:35, Luyao Huang wrote:
 After use cidr_format in function virAsprintf and vshPrintExtra, need free
 cidr_format.
 
 Fix the following memory leak from valgrind, like:
  18 bytes in 1 blocks are definitely lost in loss record 41 of 192
 at 0x4C29BBD: malloc (in 
 /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
 by 0x85CE36F: __vasprintf_chk (vasprintf_chk.c:80)
 by 0x4EE52D5: UnknownInlinedFun (stdio2.h:210)
 by 0x4EE52D5: virVasprintfInternal (virstring.c:459)
 by 0x4EE53CA: virAsprintfInternal (virstring.c:480)
 by 0x14FE96: cmdNetworkDHCPLeases (virsh-network.c:1378)
 by 0x13006B: vshCommandRun (virsh.c:1915)
 by 0x12A9E1: main (virsh.c:3699)
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  tools/virsh-network.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/tools/virsh-network.c b/tools/virsh-network.c
 index 90392d3..8ff6fd8 100644
 --- a/tools/virsh-network.c
 +++ b/tools/virsh-network.c
 @@ -1381,6 +1381,8 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
expirytime, EMPTYSTR(lease-mac),
EMPTYSTR(typestr), cidr_format,
EMPTYSTR(lease-hostname), EMPTYSTR(lease-clientid));
 +
 + VIR_FREE(cidr_format);

I didn't notice at first, but you've used a TAB to indent VIR_FREE. Our
coding style enforces use of spaces instead. Please run 'make
syntax-check before sending patches.

I'll fix the problem before pushing.
  }
  
  ret = true;
 

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] [PATCHv6 5/6] bhyve: Add console support for grub-bhyve bootloader

2014-10-30 Thread Roman Bogorodskiy

  Conrad Meyer wrote:

 This enables booting interactive GRUB menus (e.g. install CDs) with
 libvirt-bhyve.
 
 Caveat: A terminal other than the '--console' option to 'virsh start'
 (e.g. 'cu -l /dev/nmdm0B -s 115200') must be used to connect to
 grub-bhyve because the bhyve loader path is synchronous and must occur
 before the VM actually starts.
 
 Changing the bhyveProcessStart logic around to accommodate '--console'
 for interactive loader use seems like a significant project and probably
 not worth it, if UEFI/BIOS support for bhyve is coming soon.

I'm still feeling doubtful about this part. I see the impact of this
change in the following way.

 * Users, who wish to use console with grub-bhyve:
   - They still cannot use 'virsh console' for that and need to use 'cu'
 or something like that
   - They need to update to a specific grub-bhyve version (currently
 unreleased). The information about required version is not very
 easy to find, so the chance is high that most users will miss it

 * Users, who doesn't need to use console with grub-bhyve:
   - They will still need to update to the specific grub-bhyve version,
 otherwise a domain with bhyve-grub bootloader will fail with not
 very obvious from user's POV error because of the unknown argument
 to grub-bhyve

If we leave the thing as is, e.g. only have bootloader support, then we
will have:

 * Users, who with to use console with grub-bhyve:
   - Have to construct bootloader_args manually to add the --cons-dev 
   argument
   - Like in a case with an explicit support, cannot use 'virsh console'
 for the bootloader step
   - Obviously, need to update grub-bhyve

 * users, who doesn't need to use console with grub-bhyve
   - Nothing changes for them, they can use an older grub-bhyve without
 issues

Roman Bogorodskiy

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

Re: [libvirt] [PATCH] Fix memory leak in cmdNetworkDHCPLeases

2014-10-30 Thread lhuang



On 10/30/2014 02:27 PM, Peter Krempa wrote:

On 10/30/14 03:35, Luyao Huang wrote:

After use cidr_format in function virAsprintf and vshPrintExtra, need free
cidr_format.

Fix the following memory leak from valgrind, like:
  18 bytes in 1 blocks are definitely lost in loss record 41 of 192
 at 0x4C29BBD: malloc (in 
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
 by 0x85CE36F: __vasprintf_chk (vasprintf_chk.c:80)
 by 0x4EE52D5: UnknownInlinedFun (stdio2.h:210)
 by 0x4EE52D5: virVasprintfInternal (virstring.c:459)
 by 0x4EE53CA: virAsprintfInternal (virstring.c:480)
 by 0x14FE96: cmdNetworkDHCPLeases (virsh-network.c:1378)
 by 0x13006B: vshCommandRun (virsh.c:1915)
 by 0x12A9E1: main (virsh.c:3699)

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  tools/virsh-network.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 90392d3..8ff6fd8 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1381,6 +1381,8 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
expirytime, EMPTYSTR(lease-mac),
EMPTYSTR(typestr), cidr_format,
EMPTYSTR(lease-hostname), EMPTYSTR(lease-clientid));
+
+   VIR_FREE(cidr_format);

I didn't notice at first, but you've used a TAB to indent VIR_FREE. Our
coding style enforces use of spaces instead. Please run 'make
syntax-check before sending patches.

I'll fix the problem before pushing.

Oh sorry,i forgot it, thanks your advise :)

  }
  
  ret = true;



Peter



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

[libvirt] [PATCH] qemu: make advice from numad available when building commandline

2014-10-30 Thread Martin Kletzander

Particularly in qemuBuildNumaArgStr(), there was a need for the advice
due to memory backing, which needs to know the nodeset it will be pinned
to.  With newer qemu this caused the following error when starting
domain:

  error: internal error: Advice from numad is needed in case of
  automatic numa placement

even when starting perfectly valid domain, e.g.:

  ...
  vcpu placement='auto'4/vcpu
  numatune
memory mode='strict' placement='auto'/
  /numatune
  cpu
numa
  cell id='0' cpus='0' memory='524288'/
  cell id='1' cpus='1' memory='524288'/
/numa
  /cpu
  ...

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

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/qemu/qemu_command.c  | 10 ++
 src/qemu/qemu_command.h  |  3 ++-
 src/qemu/qemu_driver.c   |  3 ++-
 src/qemu/qemu_process.c  |  3 ++-
 tests/qemuxml2argvtest.c |  3 ++-
 tests/qemuxmlnstest.c|  2 +-
 6 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..207e2b0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6636,7 +6636,8 @@ static int
 qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 const virDomainDef *def,
 virCommandPtr cmd,
-virQEMUCapsPtr qemuCaps)
+virQEMUCapsPtr qemuCaps,
+virBitmapPtr nodeset)
 {
 size_t i, j;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -6796,7 +6797,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,

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

-if (virDomainNumatuneMaybeFormatNodeset(def-numatune, NULL,
+if (virDomainNumatuneMaybeFormatNodeset(def-numatune, nodeset,
 nodemask, i)  0)
 goto cleanup;

@@ -7764,7 +7765,8 @@ qemuBuildCommandLine(virConnectPtr conn,
  virNetDevVPortProfileOp vmop,
  qemuBuildCommandLineCallbacksPtr callbacks,
  bool standalone,
- bool enableFips)
+ bool enableFips,
+ virBitmapPtr nodeset)
 {
 virErrorPtr originalError = NULL;
 size_t i, j;
@@ -7992,7 +7994,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 }

 if (def-cpu  def-cpu-ncells)
-if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps)  0)
+if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset)  0)
 goto error;

 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_UUID))
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index aa40c9e..f7d3c2d 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -79,7 +79,8 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
virNetDevVPortProfileOp vmop,
qemuBuildCommandLineCallbacksPtr callbacks,
bool forXMLToArgv,
-   bool enableFips)
+   bool enableFips,
+   virBitmapPtr nodeset)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11);

 /* Generate '-device' string for chardev device */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 373daab..5ac638a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6472,7 +6472,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr 
conn,
  VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
  buildCommandLineCallbacks,
  true,
- qemuCheckFips(
+ qemuCheckFips(),
+ NULL)))
 goto cleanup;

 ret = virCommandToString(cmd);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ef258cf..6cbd608 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4362,7 +4362,8 @@ int qemuProcessStart(virConnectPtr conn,
  priv-monJSON, priv-qemuCaps,
  migrateFrom, stdin_fd, snapshot, vmop,
  buildCommandLineCallbacks, false,
- qemuCheckFips(
+ qemuCheckFips(),
+ nodemask)))
 goto cleanup;

 /* now that we know it is about to start call the hook if present */
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0e9fab9..9b8e3e2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -362,7 +362,8 @@ static int testCompareXMLToArgvFiles(const char *xml,
  migrateFrom, migrateFd, NULL,

Re: [libvirt] [PATCH v3 1/3] numatune: add check for numatune nodeset range

2014-10-30 Thread Martin Kletzander


On Thu, Oct 30, 2014 at 02:23:00AM +, Chen, Fan wrote:

On Wed, 2014-10-29 at 14:20 +0100, Martin Kletzander wrote:

On Wed, Oct 29, 2014 at 08:33:32PM +0800, Chen Fan wrote:
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
@@ -373,6 +400,12 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED,
_(NUMA isn't available on this host));
 return -1;
 }
+
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+return true;
+}
 #endif


In what case would you like this to return true?


when libvirt does not support numa, we can not check it.
maybe we should return false if setting nodeset.



That was my idea, I just wanted to make sure we're on the same page.
The thing is that if you want something that's not available, it makes
more sense to say NO then just allow it because libvirt doesn't
know.  Make the user fix it :)

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] conf: forbid negative values in virDomainParseScaledValue

2014-10-30 Thread Martin Kletzander


On Wed, Oct 29, 2014 at 04:18:42PM -0600, Eric Blake wrote:

On 10/29/2014 10:35 AM, Martin Kletzander wrote:

It makes sense for none of the callers to have negative value as an
output and, fortunately, if anyone tried defining domain with negative
memory or any other value parsed by virDomainParseScaledValue(), the
resulting value was 0.  That means we can error out during parsing as
it won't break anything.

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

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
it won't break anything -- famous last words?

 src/conf/domain_conf.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)


ACK.  Sounds like a bug fix, and therefore safe for 1.2.10



Um... on one hand it is, but on the other one this is bug that's been
around since we introduced @unit (almost forever) :-)  Anyway, the
patch is pretty small and it affects nothing else, so I pushed it.
Thanks for the review.

Martin


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

[libvirt] [PATCH] vbox: don't register NULL driver

2014-10-30 Thread Martin Kletzander

We were missing check for the fact that the storage driver was found and
in case there is no vbox storage driver available, daemon raised the
following error each start:

  error : virRegisterStorageDriver:592 : driver in
  virRegisterStorageDriver must not be NULL

Fixing this makes the condition unified with networkDriver registration
in vbox as well.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/vbox/vbox_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c
index c64d2d6..b2e35e9 100644
--- a/src/vbox/vbox_driver.c
+++ b/src/vbox/vbox_driver.c
@@ -74,7 +74,7 @@ int vboxStorageRegister(void)
 if (VBoxCGlueInit(uVersion) == 0)
 storageDriver = vboxGetStorageDriver(uVersion);

-if (virRegisterStorageDriver(storageDriver)  0)
+if (storageDriver  virRegisterStorageDriver(storageDriver)  0)
 return -1;
 return 0;
 }
-- 
2.1.2

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

Re: [libvirt] [PATCH] vbox: don't register NULL driver

2014-10-30 Thread Peter Krempa

On 10/30/14 10:03, Martin Kletzander wrote:
 We were missing check for the fact that the storage driver was found and
 in case there is no vbox storage driver available, daemon raised the
 following error each start:
 
   error : virRegisterStorageDriver:592 : driver in
   virRegisterStorageDriver must not be NULL
 
 Fixing this makes the condition unified with networkDriver registration
 in vbox as well.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/vbox/vbox_driver.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 

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 v2 1/2] util: Introduce virPidFileForceCleanupPath

2014-10-30 Thread Peter Krempa

On 10/12/14 14:12, Martin Kletzander wrote:
 This function is used to cleanup a pidfile doing whatever it takes, even
 killing the owning process.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
 v2:
  - Don't use /proc, but simply just try to acquire the pidfile.
  - https://www.redhat.com/archives/libvir-list/2014-October/msg00320.html
 
  src/libvirt_private.syms |  1 +
  src/util/virpidfile.c| 42 ++
  src/util/virpidfile.h|  2 ++
  3 files changed, 45 insertions(+)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index d6265ac..30d100d 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1810,6 +1810,7 @@ virPidFileBuildPath;
  virPidFileConstructPath;
  virPidFileDelete;
  virPidFileDeletePath;
 +virPidFileForceCleanupPath;
  virPidFileRead;
  virPidFileReadIfAlive;
  virPidFileReadPath;
 diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
 index a3b8846..a64a1cf 100644
 --- a/src/util/virpidfile.c
 +++ b/src/util/virpidfile.c
 @@ -37,6 +37,7 @@
  #include c-ctype.h
  #include areadlink.h
  #include virstring.h
 +#include virprocess.h
 
  #define VIR_FROM_THIS VIR_FROM_NONE
 
 @@ -567,3 +568,44 @@ virPidFileConstructPath(bool privileged,
  VIR_FREE(rundir);
  return ret;
  }
 +
 +
 +/**
 + * virPidFileForceCleanupPath:
 + *
 + * Check if the pidfile is left around and clean it up whatever it
 + * takes.  This doesn't raise an error.  This function must not be
 + * called multiple times with the same path, be it in threads or
 + * processes.
 + *
 + * Returns 0 if the pidfile was successfully cleaned up, -1 otherwise.

Possibly worth mentioning that this function doesn't set libvirt errors.

 + */
 +int
 +virPidFileForceCleanupPath(const char *path)

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 v2 2/2] qemu: make sure capability probing process can start

2014-10-30 Thread Peter Krempa

On 10/12/14 14:12, Martin Kletzander wrote:
 When daemon is killed right in the middle of probing a qemu binary for
 its capabilities, the VM is left running.  Next time the daemon is

s/VM/qemu process/ ? The qemu isn't running anything so it might confuse
someone.

 starting, it cannot start qemu process because the one that's already
 running does have the pidfile flock()'d.
 
 Reported-by: Wang Yufei james.wangyu...@huawei.com
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
 v2:
  - Don't use /proc, but simply just try to acquire the pidfile.
  - https://www.redhat.com/archives/libvir-list/2014-October/msg00320.html
 
  src/qemu/qemu_capabilities.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
 index 6fcb5c7..8aedf3f 100644
 --- a/src/qemu/qemu_capabilities.c
 +++ b/src/qemu/qemu_capabilities.c
 @@ -3243,6 +3243,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
  config.data.nix.path = monpath;
  config.data.nix.listen = false;
 
 +virPidFileForceCleanupPath(pidfile);
 +
  VIR_DEBUG(Try to get caps via QMP qemuCaps=%p, qemuCaps);
 
  /*

ACK. It is a bugfix, but the code path is is critical. I'm leaning
towards pushing this before the freeze but I'd feel better with yet
another opinion.

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] Teach virt-aa-helper to use TEMPLATE.qemu if the domain is kvm or kqemu

2014-10-30 Thread Peter Krempa

On 10/29/14 14:31, Serge Hallyn wrote:
 Quoting Cédric Bosdonnat (cbosdon...@suse.com):
 Without this patch, kvm and kqemu domains confined with apparmor can't start
 due to virt-aa-helper not finding TEMPLATE.kvm or TEMPLATE.kqemu. This patch
 points all kvm-related drivers to TEMPLATE.qemu.
 
 D'oh, I dropped the ball here.  I had a patch like this but it seems it never
 made it to the list.
 
 Thanks, Cédric.
 
 Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com

Thanks for confirming it's the right thing to do with apparmor. I'll
push this patch in a moment. I was about to ACK it but was afraid to do
so as I don't use apparmor actually.

Peter

 

 ---
  src/security/virt-aa-helper.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

 diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
 index 9afc8db..6b95fdb 100644
 --- a/src/security/virt-aa-helper.c
 +++ b/src/security/virt-aa-helper.c
 @@ -341,15 +341,25 @@ create_profile(const char *profile, const char 
 *profile_name,
  int tlen, plen;
  int fd;
  int rc = -1;
 +const char *driver_name = NULL;
  
  if (virFileExists(profile)) {
  vah_error(NULL, 0, _(profile exists));
  goto end;
  }
  
 +switch (virtType) {
 +case VIR_DOMAIN_VIRT_QEMU: 
 +case VIR_DOMAIN_VIRT_KQEMU: 
 +case VIR_DOMAIN_VIRT_KVM:
 +driver_name = qemu;
 +break;
 +default:
 +driver_name = virDomainVirtTypeToString(virtType);
 +}
  
  if (virAsprintfQuiet(template, %s/TEMPLATE.%s, APPARMOR_DIR 
 /libvirt,
 - virDomainVirtTypeToString(virtType))  0) {
 + driver_name)  0) {
  vah_error(NULL, 0, _(template name exceeds maximum length));
  goto end;
  }
 -- 
 1.8.4.5

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




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] Teach virt-aa-helper to use TEMPLATE.qemu if the domain is kvm or kqemu

2014-10-30 Thread Peter Krempa

On 10/30/14 10:47, Peter Krempa wrote:
 On 10/29/14 14:31, Serge Hallyn wrote:
 Quoting Cédric Bosdonnat (cbosdon...@suse.com):
 Without this patch, kvm and kqemu domains confined with apparmor can't start
 due to virt-aa-helper not finding TEMPLATE.kvm or TEMPLATE.kqemu. This patch
 points all kvm-related drivers to TEMPLATE.qemu.

 D'oh, I dropped the ball here.  I had a patch like this but it seems it never
 made it to the list.

 Thanks, Cédric.

 Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com
 
 Thanks for confirming it's the right thing to do with apparmor. I'll
 push this patch in a moment. I was about to ACK it but was afraid to do
 so as I don't use apparmor actually.

Ah, it actually is already pushed and I didn't notice. Sorry for the noise.

 
 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] ceps-based backing stores

2014-10-30 Thread Peter Krempa

On 10/30/14 01:15, Lyssa P. Livingston wrote:
 I have a file with an ceph-based backing store.  I run this command:
 
 nova boot vm-clone
 
 where vm-clone is the rbd-backed file.  It generates this command:
 
 qemu-img create -f qcow2 -b 
 rbd:rbdpool/disk/instance-0002.0.disk:conf=/etc/ceph/ceph.conf:id=admin 
 -F raw /mnt/novadisk/nova/instances/3efa8a7b-75a8-4695-8fdd-659185c46b7d/disk
 
 and I am getting this error:
 
 error: internal error: backing store parser is not implemented for 
 protocol rbd

Libvirt is parsing the information saved in the backing store field of
an image to re-create the backing chain structure in memory.
Unfortunately I didn't implement all the possible formats for this as
there's a lot of them and one of the missing pieces is the colon syntax
for RBD volumes.

I'll try to come up with a patch for this that will allow to recognize
the RBD volume as a remote one where libvirt isn't able to access it and
allow to start the VM.

 
 I was under the impression that qemu could work with ceph-based backing 
 stores, so is there something I should run first, or files that I should 
 patch, or ... ???  I am running libvirt 1.2.9.  Thanks for any assistance you 
 can provide.

Qemu is able to handle those. Libvirt currently doesn't support the syntax.

 
   Lyssa
 


Peter

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




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

Re: [libvirt] [Nbd] [Qemu-devel] spec, RFC: TLS support for NBDµ

2014-10-30 Thread Stefan Hajnoczi

On Sat, Oct 25, 2014 at 12:43:35PM +0200, Wouter Verhelst wrote:
 I haven't seen a reply to this anymore. Do people still have comments?
 I'm planning on doing a release of nbd later this weekend, and would
 like to include this (not the TLS implementation yet, but at least the
 spec)

Hi Wouter,
From https://github.com/yoe/nbd/blob/tlsspec/doc/proto.txt:

  * NBD_OPT_STARTTLS (5)

  The client wishes to initiate TLS. XXX Data.

Is there text missing for XXX Data?

Also, I suggest at least developing a prototype before releasing the
specification changes.  Issues that were unknown ahead of time might be
discovered during development.  Why the rush to release specification
changes?

Stefan


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

[libvirt] [PATCH] Reject live update of offloading options

2014-10-30 Thread Ján Tomko

https://bugzilla.redhat.com/show_bug.cgi?id=1155441
---
 src/qemu/qemu_hotplug.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 33241fb..d3bf392 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1981,7 +1981,18 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
  olddev-driver.virtio.txmode != newdev-driver.virtio.txmode ||
  olddev-driver.virtio.ioeventfd != newdev-driver.virtio.ioeventfd ||
  olddev-driver.virtio.event_idx != newdev-driver.virtio.event_idx ||
- olddev-driver.virtio.queues != newdev-driver.virtio.queues)) {
+ olddev-driver.virtio.queues != newdev-driver.virtio.queues ||
+ olddev-driver.virtio.host.csum != newdev-driver.virtio.host.csum ||
+ olddev-driver.virtio.host.gso != newdev-driver.virtio.host.gso ||
+ olddev-driver.virtio.host.tso4 != newdev-driver.virtio.host.tso4 ||
+ olddev-driver.virtio.host.tso6 != newdev-driver.virtio.host.tso6 ||
+ olddev-driver.virtio.host.ecn != newdev-driver.virtio.host.ecn ||
+ olddev-driver.virtio.host.ufo != newdev-driver.virtio.host.ufo ||
+ olddev-driver.virtio.guest.csum != newdev-driver.virtio.guest.csum 
||
+ olddev-driver.virtio.guest.tso4 != newdev-driver.virtio.guest.tso4 
||
+ olddev-driver.virtio.guest.tso6 != newdev-driver.virtio.guest.tso6 
||
+ olddev-driver.virtio.guest.ecn != newdev-driver.virtio.guest.ecn ||
+ olddev-driver.virtio.guest.ufo != newdev-driver.virtio.guest.ufo)) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(cannot modify virtio network device driver 
attributes));
 goto cleanup;
-- 
2.0.4

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

Re: [libvirt] [PATCHv6 5/6] bhyve: Add console support for grub-bhyve bootloader

2014-10-30 Thread Conrad Meyer

On Thu, Oct 30, 2014 at 2:29 AM, Roman Bogorodskiy
bogorods...@gmail.com wrote:
   Conrad Meyer wrote:

 This enables booting interactive GRUB menus (e.g. install CDs) with
 libvirt-bhyve.

 Caveat: A terminal other than the '--console' option to 'virsh start'
 (e.g. 'cu -l /dev/nmdm0B -s 115200') must be used to connect to
 grub-bhyve because the bhyve loader path is synchronous and must occur
 before the VM actually starts.

 Changing the bhyveProcessStart logic around to accommodate '--console'
 for interactive loader use seems like a significant project and probably
 not worth it, if UEFI/BIOS support for bhyve is coming soon.

 I'm still feeling doubtful about this part. I see the impact of this
 change in the following way.

  * Users, who wish to use console with grub-bhyve:
- They still cannot use 'virsh console' for that and need to use 'cu'
  or something like that
- They need to update to a specific grub-bhyve version (currently
  unreleased). The information about required version is not very
  easy to find, so the chance is high that most users will miss it

  * Users, who doesn't need to use console with grub-bhyve:
- They will still need to update to the specific grub-bhyve version,
  otherwise a domain with bhyve-grub bootloader will fail with not
  very obvious from user's POV error because of the unknown argument
  to grub-bhyve

 If we leave the thing as is, e.g. only have bootloader support, then we
 will have:

  * Users, who with to use console with grub-bhyve:
- Have to construct bootloader_args manually to add the --cons-dev
argument
- Like in a case with an explicit support, cannot use 'virsh console'
  for the bootloader step
- Obviously, need to update grub-bhyve

  * users, who doesn't need to use console with grub-bhyve
- Nothing changes for them, they can use an older grub-bhyve without
  issues

 Roman Bogorodskiy

Hi,

I will improve patch 5 to at least do detection and resubmit 5-6. In
the mean time, patches 1-4 do not depend on 5-6 and can stand on their
own. I believe critiques brought up in earlier reviews have been
addressed.

Thanks,
Conrad

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

Re: [libvirt] [PATCH 14/19] vbox: Rewrite vboxStorageVolDelete

2014-10-30 Thread John Ferlan



On 10/23/2014 09:46 PM, Taowei Luo wrote:
 The API on IHardDiskAttachment is merged into IMediumAttachment.
 So, we don't need it anymore.
 ---
  src/vbox/vbox_storage.c   |  160 
  src/vbox/vbox_tmpl.c  |  202 
 ++---
  src/vbox/vbox_uniformed_api.h |6 ++
  3 files changed, 192 insertions(+), 176 deletions(-)
 

The Coverity checker had some issues with this patch. Coverity has a
single UNUSED_VALUE for 5 different checks. It wasn't initially clear
what the problem was, but I was able to at least determine that although
I'm not quite sure of the correct fix/patch.


 diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
 index 1e6ca67..45090eb 100644
 --- a/src/vbox/vbox_storage.c
 +++ b/src/vbox/vbox_storage.c
 @@ -530,3 +530,163 @@ virStorageVolPtr 
 vboxStorageVolCreateXML(virStoragePoolPtr pool,
  virStorageVolDefFree(def);
  return ret;
  }
 +
 +int vboxStorageVolDelete(virStorageVolPtr vol, unsigned int flags)
 +{
 +vboxGlobalData *data = vol-conn-privateData;
 +unsigned char uuid[VIR_UUID_BUFLEN];
 +IHardDisk *hardDisk = NULL;
 +int deregister = 0;
 +PRUint32 hddstate = 0;
 +size_t i = 0;
 +size_t j = 0;
 +PRUint32  machineIdsSize = 0;
 +vboxArray machineIds = VBOX_ARRAY_INITIALIZER;
 +vboxIIDUnion hddIID;
 +nsresult rc;
 +int ret = -1;
 +
 +if (!data-vboxObj) {
 +return ret;
 +}
 +
 +VBOX_IID_INITIALIZE(hddIID);
 +virCheckFlags(0, -1);
 +
 +if (virUUIDParse(vol-key, uuid)  0) {
 +virReportError(VIR_ERR_INVALID_ARG,
 +   _(Could not parse UUID from '%s'), vol-key);
 +return -1;
 +}
 +
 +vboxIIDFromUUID(hddIID, uuid);
 +rc = gVBoxAPI.UIVirtualBox.GetHardDiskByIID(data-vboxObj, hddIID, 
 hardDisk);
 +if (NS_FAILED(rc))
 +goto cleanup;
 +
 +gVBoxAPI.UIMedium.GetState(hardDisk, hddstate);
 +if (hddstate == MediaState_Inaccessible)
 +goto cleanup;
 +
 +gVBoxAPI.UArray.vboxArrayGet(machineIds, hardDisk,
 + 
 gVBoxAPI.UArray.handleMediumGetMachineIds(hardDisk));
 +
 +#if defined WIN32
 +/* VirtualBox 2.2 on Windows represents IIDs as GUIDs and the
 + * machineIds array contains direct instances of the GUID struct
 + * instead of pointers to the actual struct instances. But there
 + * is no 128bit width simple item type for a SafeArray to fit a
 + * GUID in. The largest simple type it 64bit width and VirtualBox
 + * uses two of this 64bit items to represents one GUID. Therefore,
 + * we divide the size of the SafeArray by two, to compensate for
 + * this workaround in VirtualBox */
 +if (gVBoxAPI.uVersion = 2001052  gVBoxAPI.uVersion  2002051)
 +machineIds.count /= 2;
 +#endif /* !defined WIN32 */
 +
 +machineIdsSize = machineIds.count;
 +
 +for (i = 0; i  machineIds.count; i++) {
 +IMachine *machine = NULL;
 +vboxIIDUnion machineId;
 +vboxArray hddAttachments = VBOX_ARRAY_INITIALIZER;
 +
 +VBOX_IID_INITIALIZE(machineId);
 +vboxIIDFromArrayItem(machineId, machineIds, i);
 +
 +if (gVBoxAPI.getMachineForSession) {
 +rc = gVBoxAPI.UIVirtualBox.GetMachine(data-vboxObj, machineId, 
 machine);

Event value_overwrite:  Value from
(*gVBoxAPI.UIMachine.SaveSettings)(machine) is overwritten with value
from (*gVBoxAPI.UIVirtualBox.GetMachine)(data-vboxObj, machineId,
machine).

 +if (NS_FAILED(rc)) {
 +virReportError(VIR_ERR_NO_DOMAIN, %s,
 +   _(no domain with matching uuid));
 +break;
 +}
 +}
 +
 +rc = gVBoxAPI.UISession.Open(data, machineId, machine);

Event value_overwrite:  Value from
(*gVBoxAPI.UIMachine.SaveSettings)(machine) is overwritten with value
from (*gVBoxAPI.UISession.Open)(data, machineId, machine).

 +
 +if (NS_FAILED(rc)) {
 +vboxIIDUnalloc(machineId);
 +continue;
 +}
 +
 +rc = gVBoxAPI.UISession.GetMachine(data-vboxSession, machine);
 +if (NS_FAILED(rc))
 +goto cleanupLoop;
 +
 +gVBoxAPI.UArray.vboxArrayGet(hddAttachments, machine,
 + 
 gVBoxAPI.UArray.handleMachineGetMediumAttachments(machine));
 +
 +for (j = 0; j  hddAttachments.count; j++) {
 +IMediumAttachment *hddAttachment = hddAttachments.items[j];
 +IHardDisk *hdd = NULL;
 +vboxIIDUnion iid;
 +
 +if (!hddAttachment)
 +continue;
 +
 +rc = gVBoxAPI.UIMediumAttachment.GetMedium(hddAttachment, hdd);

Event value_overwrite:  Value from
(*gVBoxAPI.UIMachine.SaveSettings)(machine) is overwritten with value
from (*gVBoxAPI.UIMediumAttachment.GetMedium)(hddAttachment, hdd).

 +if (NS_FAILED(rc) || !hdd)
 +continue;
 +
 +

Re: [libvirt] [PATCH] Reject live update of offloading options

2014-10-30 Thread Eric Blake

On 10/30/2014 06:38 AM, Ján Tomko wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1155441
 ---
  src/qemu/qemu_hotplug.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

ACK. Safe during the freeze.

 
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 33241fb..d3bf392 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -1981,7 +1981,18 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
   olddev-driver.virtio.txmode != newdev-driver.virtio.txmode ||
   olddev-driver.virtio.ioeventfd != newdev-driver.virtio.ioeventfd 
 ||
   olddev-driver.virtio.event_idx != newdev-driver.virtio.event_idx 
 ||
 - olddev-driver.virtio.queues != newdev-driver.virtio.queues)) {
 + olddev-driver.virtio.queues != newdev-driver.virtio.queues ||
 + olddev-driver.virtio.host.csum != newdev-driver.virtio.host.csum 
 ||
 + olddev-driver.virtio.host.gso != newdev-driver.virtio.host.gso ||
 + olddev-driver.virtio.host.tso4 != newdev-driver.virtio.host.tso4 
 ||
 + olddev-driver.virtio.host.tso6 != newdev-driver.virtio.host.tso6 
 ||
 + olddev-driver.virtio.host.ecn != newdev-driver.virtio.host.ecn ||
 + olddev-driver.virtio.host.ufo != newdev-driver.virtio.host.ufo ||
 + olddev-driver.virtio.guest.csum != 
 newdev-driver.virtio.guest.csum ||
 + olddev-driver.virtio.guest.tso4 != 
 newdev-driver.virtio.guest.tso4 ||
 + olddev-driver.virtio.guest.tso6 != 
 newdev-driver.virtio.guest.tso6 ||
 + olddev-driver.virtio.guest.ecn != newdev-driver.virtio.guest.ecn 
 ||
 + olddev-driver.virtio.guest.ufo != 
 newdev-driver.virtio.guest.ufo)) {
  virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
 _(cannot modify virtio network device driver 
 attributes));
  goto cleanup;
 

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



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

[libvirt] [PATCH 1/3] virsh: don't list unknown domains

2014-10-30 Thread Martin Kletzander

When the list of domains is fetched and being printed, but in the
meantime one domain was undefined before its status was fetched, the
output then includes domain with no state.  With this patch, such
domain is skipped over as consecutive 'virsh list --all' (or the same
one ran a second later) wouldn't list it anyway.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tools/virsh-domain-monitor.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 2af0d4f..4e434f8 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1916,6 +1916,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
 ignore_value(virStrcpyStatic(id_buf, -));

 state = vshDomainState(ctl, dom, NULL);
+
+/* Domain could've been removed in the meantime */
+if (state  0)
+continue;
+
 if (optTable  managed  state == VIR_DOMAIN_SHUTOFF 
 virDomainHasManagedSaveImage(dom, 0)  0)
 state = -2;
-- 
2.1.2

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

[libvirt] [PATCH 3/3] Avoid rare race when undefining domain

2014-10-30 Thread Martin Kletzander

When one domain is being undefined and at the same time started, for
example, there is a possibility of a rare problem occuring.

 - Thread 1 does virDomainUndefine(), has the lock, checks that the
   domain is active and because it's not, calls
   virDomainObjListRemove().

 - Thread 2 does virDomainCreate() and tries to lock the domain.

 - Thread 1 needs to lock domain list in order to remove the domain from
   it, but must unlock domain first (proper order is to lock domain list
   first and the domain itself second).

 - Thread 2 grabs the lock, starts the domain and releases the lock.

 - Thread 1 grabs the lock and removes the domain from list.

With this patch:

 - The undefining domain gets marked as to undefine before it is
   unlocked.

 - If domain is found in any of the search APIs, it's returned only if
   it is not marked as to undefine.  The check is done while the
   domain is locked.

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

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/conf/domain_conf.c | 23 ---
 src/conf/domain_conf.h |  1 +
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 43574e1..b92a58a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1054,8 +1054,13 @@ virDomainObjPtr 
virDomainObjListFindByID(virDomainObjListPtr doms,
 virDomainObjPtr obj;
 virObjectLock(doms);
 obj = virHashSearch(doms-objs, virDomainObjListSearchID, id);
-if (obj)
+if (obj) {
 virObjectLock(obj);
+if (obj-undefining) {
+virObjectUnlock(obj);
+obj = NULL;
+}
+}
 virObjectUnlock(doms);
 return obj;
 }
@@ -1071,8 +1076,13 @@ virDomainObjPtr 
virDomainObjListFindByUUID(virDomainObjListPtr doms,
 virUUIDFormat(uuid, uuidstr);

 obj = virHashLookup(doms-objs, uuidstr);
-if (obj)
+if (obj) {
 virObjectLock(obj);
+if (obj-undefining) {
+virObjectUnlock(obj);
+obj = NULL;
+}
+}
 virObjectUnlock(doms);
 return obj;
 }
@@ -1097,8 +1107,13 @@ virDomainObjPtr 
virDomainObjListFindByName(virDomainObjListPtr doms,
 virDomainObjPtr obj;
 virObjectLock(doms);
 obj = virHashSearch(doms-objs, virDomainObjListSearchName, name);
-if (obj)
+if (obj) {
 virObjectLock(obj);
+if (obj-undefining) {
+virObjectUnlock(obj);
+obj = NULL;
+}
+}
 virObjectUnlock(doms);
 return obj;
 }
@@ -2538,6 +2553,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms,

 virUUIDFormat(dom-def-uuid, uuidstr);
 virObjectRef(dom);
+dom-undefining = true;
 virObjectUnlock(dom);

 VIR_WARN(NOW!!!);
@@ -2563,6 +2579,7 @@ void virDomainObjListRemoveLocked(virDomainObjListPtr 
doms,
 char uuidstr[VIR_UUID_STRING_BUFLEN];

 virUUIDFormat(dom-def-uuid, uuidstr);
+dom-undefining = true;
 virObjectUnlock(dom);

 virHashRemoveEntry(doms-objs, uuidstr);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9908d88..1d28ed0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2155,6 +2155,7 @@ struct _virDomainObj {
 unsigned int autostart : 1;
 unsigned int persistent : 1;
 unsigned int updated : 1;
+unsigned int undefining : 1; /* the domain is about to be undefined */

 virDomainDefPtr def; /* The current definition */
 virDomainDefPtr newDef; /* New definition to activate at shutdown */
-- 
2.1.2

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

[libvirt] [PATCH 0/3] Avoid rare race with virDomainUndefine()

2014-10-30 Thread Martin Kletzander

First patch just fixes 'virsh list' for a rare case that is similar to
the one fixed in the third patch.  Second patch is just a helper for
reproducing the race.

This was found in qemu driver but is fixed on upper level, so it
should be more general.

Martin Kletzander (3):
  virsh: don't list unknown domains
  DO NOT APPLY UPSTREAM: Reproducer helper
  Avoid rare race when undefining domain

 src/conf/domain_conf.c   | 26 +++---
 src/conf/domain_conf.h   |  1 +
 tools/virsh-domain-monitor.c |  5 +
 3 files changed, 29 insertions(+), 3 deletions(-)

--
2.1.2

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

[libvirt] [PATCH 2/3] DO NOT APPLY UPSTREAM: Reproducer helper

2014-10-30 Thread Martin Kletzander

To reproduce the problem that this series is trying to fix, do the
following:

 - Term 1:
   LIBVIRT_DEBUG=2 LIBVIRT_LOG_OUTPUTS='2:stderr' daemon/libvirtd

 - Term 2:
   virsh undefine $dom

 - In term 1 look for:
   NOW!!!

 - Term 3:
   virsh start $dom

 - Enjoy:
   domain is started, but not in any list (defined nor running)

systemd prevents starting domain with the same name again with:
error: CreateMachine: File exists, but that's not very descriptive.

Beware: 'make check' fails with this patch!

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/conf/domain_conf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a351382..43574e1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2540,6 +2540,9 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
 virObjectRef(dom);
 virObjectUnlock(dom);

+VIR_WARN(NOW!!!);
+sleep(10);
+
 virObjectLock(doms);
 virObjectLock(dom);
 virHashRemoveEntry(doms-objs, uuidstr);
-- 
2.1.2

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

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

2014-10-30 Thread Erik Skultety

Patch 43b67f2e disallowed network tuning only with qemu driver, however
this patch moved the check for root privileges into
virNetDevBandwidthSetInternal wrapper function, so the call should now
fail in all possible cases. The reason to create another wrapper is that
we do execute a test suite with user privileges during compilation.
Tests fail unless the wrapper is used.
---
 src/libvirt_private.syms  |  4 
 src/util/virnetdevbandwidth.c | 31 ---
 src/util/virnetdevbandwidthpriv.h | 36 
 tests/virnetdevbandwidthtest.c|  5 +++--
 4 files changed, 67 insertions(+), 9 deletions(-)
 create mode 100644 src/util/virnetdevbandwidthpriv.h

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9d5c814..ab3d6d0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1647,6 +1647,10 @@ virNetDevBandwidthUnplug;
 virNetDevBandwidthUpdateRate;
 
 
+# util/virnetdevbandwidthpriv.h
+virNetDevBandwidthSetInternal;
+
+
 # util/virnetdevbridge.h
 virNetDevBridgeAddPort;
 virNetDevBridgeCreate;
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 5fa231a..1fc6fdf 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -21,8 +21,9 @@
  */
 
 #include config.h
-
-#include virnetdevbandwidth.h
+#include unistd.h
+#define __VIR_BANDWIDTH_PRIV_H_ALLOW__
+#include virnetdevbandwidthpriv.h
 #include vircommand.h
 #include viralloc.h
 #include virerror.h
@@ -41,9 +42,8 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
 VIR_FREE(def);
 }
 
-
 /**
- * virNetDevBandwidthSet:
+ * virNetDevBandwidthSetInternal:
  * @ifname: on which interface
  * @bandwidth: rates to set (may be NULL)
  * @hierarchical_class: whether to create hierarchical class
@@ -58,9 +58,9 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
  * Return 0 on success, -1 otherwise.
  */
 int
-virNetDevBandwidthSet(const char *ifname,
-  virNetDevBandwidthPtr bandwidth,
-  bool hierarchical_class)
+virNetDevBandwidthSetInternal(const char *ifname,
+  virNetDevBandwidthPtr bandwidth,
+  bool hierarchical_class)
 {
 int ret = -1;
 virCommandPtr cmd = NULL;
@@ -242,6 +242,23 @@ virNetDevBandwidthSet(const char *ifname,
 return ret;
 }
 
+int
+virNetDevBandwidthSet(const char *ifname,
+  virNetDevBandwidthPtr bandwidth,
+  bool hierarchical_class)
+{
+if (geteuid() != 0) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(Network bandwidth tuning is not available
+  in session mode));
+return -1;
+} else {
+return virNetDevBandwidthSetInternal(ifname,
+ bandwidth,
+ hierarchical_class);
+}
+}
+
 /**
  * virNetDevBandwidthClear:
  * @ifname: on which interface
diff --git a/src/util/virnetdevbandwidthpriv.h 
b/src/util/virnetdevbandwidthpriv.h
new file mode 100644
index 000..9c3a0fa
--- /dev/null
+++ b/src/util/virnetdevbandwidthpriv.h
@@ -0,0 +1,36 @@
+/*
+ * virnetdevbandwidthpriv.h: Functions for testing virNetDevBandwidth APIs
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ */
+
+#ifndef __VIR_BANDWIDTH_PRIV_H_ALLOW__
+# error virnetdevbandwidthpriv.h may only be included by virnetdevbandwidth.c 
or test suites
+#endif
+
+#ifndef __VIR_NETDEVBANDWIDTH_PRIV_H__
+# define __VIR_NETDEVBANDWIDTH_PRIV_H__
+
+# include virnetdevbandwidth.h
+
+int virNetDevBandwidthSetInternal(const char *ifname,
+  virNetDevBandwidthPtr bandwidth,
+  bool hierarchical_class)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
+#endif /* __VIR_NETDEVBANDWIDTH_PRIV_H__ */
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 384991e..a93d0ae 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -23,7 +23,8 @@
 #include testutils.h
 #define __VIR_COMMAND_PRIV_H_ALLOW__
 #include vircommandpriv.h
-#include virnetdevbandwidth.h
+#define __VIR_BANDWIDTH_PRIV_H_ALLOW__
+#include virnetdevbandwidthpriv.h

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

2014-10-30 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
(excluding NUMA!) 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  |  6 --
 2 files changed, 17 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..be8e335 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7835,17 +7835,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 373daab..631cb5f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10437,12 +10437,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/2] network: bandwidth tuning in session mode revert patch

2014-10-30 Thread Erik Skultety

As Laine noted correctly, we should be testing for privileges on a global scale,
when trying to set bandwidth parameters. Therefore I moved the check from 
qemu_driver.c
and qemu_command.c into virnetdevbandwidth.c

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

 src/libvirt_private.syms  |  4 
 src/qemu/qemu_command.c   | 11 ---
 src/qemu/qemu_driver.c|  6 --
 src/util/virnetdevbandwidth.c | 31 ---
 src/util/virnetdevbandwidthpriv.h | 36 
 tests/virnetdevbandwidthtest.c|  5 +++--
 6 files changed, 67 insertions(+), 26 deletions(-)
 create mode 100644 src/util/virnetdevbandwidthpriv.h

-- 
1.9.3

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

Re: [libvirt] [PATCH V2 0/2] libxl: user-specified domain config improvements

2014-10-30 Thread Jim Fehlig

Michal Privoznik wrote:
 On 11.10.2014 00:03, Jim Fehlig wrote:
 V2 of a small series to improve libxl driver's support for
 user-specified
 config

 https://www.redhat.com/archives/libvir-list/2014-September/msg01243.html

 Patches 1-4 of that series have been pushed.  In patch 5, Michal
 suggested checking that the user-specified emulator is executable.
 Patch 1 of this series is a respin of patch 5 that includes Michal's
 suggestion.  Patch 2 fixes an issue found while testing.

 Jim Fehlig (2):
libxl: Support user-specified emulator
libxl: fix double-free of libxl_domain_build_info

   src/libxl/libxl_conf.c | 48
 +---
   1 file changed, 33 insertions(+), 15 deletions(-)


 ACK

Is it ok to push these during 1.2.10 freeze?  2/2 is a bug fix...

Apologies for not promptly pushing when ACK'ed, prior to the freeze.

Regards,
Jim

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

[libvirt] [PATCH] conf: use proper maximum for parsing memory values

2014-10-30 Thread Martin Kletzander

The value is stored in unsigned long long, so ULLONG_MAX is the proper
upper limit to use.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Even though this is a build-breaker (for 32bit systems
memtune-unlimited fails to parse), I'm not pushing it as one because
it feels odd that such change wouldn't break anything else.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a351382..beb3d26 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6388,16 +6388,10 @@ virDomainParseMemory(const char *xpath, 
xmlXPathContextPtr ctxt,
  unsigned long long *mem, bool required)
 {
 int ret = -1;
-unsigned long long bytes, max;
-
-/* 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;
+unsigned long long bytes;

-ret = virDomainParseScaledValue(xpath, ctxt, bytes, 1024, max, required);
+ret = virDomainParseScaledValue(xpath, ctxt, bytes, 1024,
+ULLONG_MAX, required);
 if (ret  0)
 goto cleanup;

--
2.1.2

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

[libvirt] [PATCHv7 6/7] bhyve: Add console support for grub-bhyve bootloader

2014-10-30 Thread Conrad Meyer

This enables booting interactive GRUB menus (e.g. install CDs) with
libvirt-bhyve.

Caveat: A terminal other than the '--console' option to 'virsh start'
(e.g. 'cu -l /dev/nmdm0B -s 115200') must be used to connect to
grub-bhyve because the bhyve loader path is synchronous and must occur
before the VM actually starts.

Changing the bhyveProcessStart logic around to accommodate '--console'
for interactive loader use seems like a significant project and probably
not worth it, if UEFI/BIOS support for bhyve is coming soon.
---
 src/bhyve/bhyve_command.c | 18 ++
 src/bhyve/bhyve_driver.c  | 13 +
 src/bhyve/bhyve_driver.h  |  2 ++
 src/bhyve/bhyve_utils.h   |  2 ++
 4 files changed, 35 insertions(+)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 203495c..26d4797 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -25,8 +25,10 @@
 #include net/if.h
 #include net/if_tap.h
 
+#include bhyve_capabilities.h
 #include bhyve_command.h
 #include bhyve_domain.h
+#include bhyve_driver.h
 #include datatypes.h
 #include viralloc.h
 #include virfile.h
@@ -447,6 +449,22 @@ virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def,
 virCommandAddArgFormat(cmd, %llu,
VIR_DIV_UP(def-mem.max_balloon, 1024));
 
+if ((bhyveDriverGetGrubCaps(conn)  BHYVE_GRUB_CAP_CONSDEV) != 0 
+def-nserials  0) {
+virDomainChrDefPtr chr;
+
+chr = def-serials[0];
+
+if (chr-source.type != VIR_DOMAIN_CHR_TYPE_NMDM) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(only nmdm console types are supported));
+return NULL;
+}
+
+virCommandAddArg(cmd, --cons-dev);
+virCommandAddArg(cmd, chr-source.data.file.path);
+}
+
 /* VM name */
 virCommandAddArg(cmd, def-name);
 
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 4aee249..3820737 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1169,6 +1169,9 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED,
 if (!(bhyve_driver-caps = virBhyveCapsBuild()))
 goto cleanup;
 
+if (virBhyveProbeGrubCaps(bhyve_driver-grubcaps)  0)
+goto cleanup;
+
 if (!(bhyve_driver-xmlopt = 
virDomainXMLOptionNew(virBhyveDriverDomainDefParserConfig,

virBhyveDriverPrivateDataCallbacks,
NULL)))
@@ -1226,6 +1229,16 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED,
 return -1;
 }
 
+unsigned
+bhyveDriverGetGrubCaps(virConnectPtr conn)
+{
+bhyveConnPtr driver = conn-privateData;
+
+if (driver != NULL)
+return driver-grubcaps;
+return 0;
+}
+
 static void
 bhyveStateAutoStart(void)
 {
diff --git a/src/bhyve/bhyve_driver.h b/src/bhyve/bhyve_driver.h
index b70991d..af2424a 100644
--- a/src/bhyve/bhyve_driver.h
+++ b/src/bhyve/bhyve_driver.h
@@ -25,4 +25,6 @@
 
 int bhyveRegister(void);
 
+unsigned bhyveDriverGetGrubCaps(virConnectPtr conn);
+
 #endif /* __BHYVE_DRIVER_H__ */
diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h
index 848f9a1..bbaa3a3 100644
--- a/src/bhyve/bhyve_utils.h
+++ b/src/bhyve/bhyve_utils.h
@@ -45,6 +45,8 @@ struct _bhyveConn {
 virObjectEventStatePtr domainEventState;
 
 virCloseCallbacksPtr closeCallbacks;
+
+unsigned grubcaps;
 };
 
 typedef struct _bhyveConn bhyveConn;
-- 
1.9.3

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

Re: [libvirt] [PATCH] conf: use proper maximum for parsing memory values

2014-10-30 Thread Eric Blake

On 10/30/2014 09:49 AM, Martin Kletzander wrote:
 The value is stored in unsigned long long, so ULLONG_MAX is the proper
 upper limit to use.

No, it's not.

 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
 Even though this is a build-breaker (for 32bit systems
 memtune-unlimited fails to parse), I'm not pushing it as one because
 it feels odd that such change wouldn't break anything else.

What's the compile failure?  This patch is intentionally trying to fit
the largest value that will fit in an unsigned long when scaled by
kilobytes, while still parsing by bytes.  Thus, on 64-bit machines, you
can parse 0x bytes, then scale down by 1024 and store in
unsigned long; on 32-bit machines, your maximum has to be limited to
0x*1024 bytes before scaling back down.

 
  src/conf/domain_conf.c | 12 +++-
  1 file changed, 3 insertions(+), 9 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index a351382..beb3d26 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -6388,16 +6388,10 @@ virDomainParseMemory(const char *xpath, 
 xmlXPathContextPtr ctxt,
   unsigned long long *mem, bool required)
  {
  int ret = -1;
 -unsigned long long bytes, max;
 -
 -/* 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;
 +unsigned long long bytes;
 
 -ret = virDomainParseScaledValue(xpath, ctxt, bytes, 1024, max, 
 required);
 +ret = virDomainParseScaledValue(xpath, ctxt, bytes, 1024,
 +ULLONG_MAX, required);

NACK.  I need to see the compiler failure to see the proper fix, but
this fix is not going to work.


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



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

[libvirt] [PATCHv7 3/7] domaincommon.rng: Add 'bootloader' to os=hvm schema for Bhyve

2014-10-30 Thread Conrad Meyer

Additionally, make the bootloader tag optional (for bhyveload with
custom arguments) (also, matches the actual parser).
---
 docs/schemas/domaincommon.rng | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 20d81ae..87ba888 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -234,6 +234,9 @@
 /choice
   /define
   define name=oshvm
+optional
+  ref name=bootloader/
+/optional
 element name=os
   ref name=ostypehvm/
   interleave
@@ -1054,12 +1057,14 @@
 --
   define name=bootloader
 interleave
-  element name=bootloader
-choice
-  ref name=absFilePath/
-  empty/
-/choice
-  /element
+  optional
+element name=bootloader
+  choice
+ref name=absFilePath/
+empty/
+  /choice
+/element
+  /optional
   optional
 element name=bootloader_args
   text/
-- 
1.9.3

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

[libvirt] [PATCHv7 5/7] bhyve: Probe grub-bhyve for --cons-dev capability

2014-10-30 Thread Conrad Meyer

---
 src/bhyve/bhyve_capabilities.c | 37 +
 src/bhyve/bhyve_capabilities.h |  3 +++
 2 files changed, 40 insertions(+)

diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
index 132ce91..6e9a943 100644
--- a/src/bhyve/bhyve_capabilities.c
+++ b/src/bhyve/bhyve_capabilities.c
@@ -23,6 +23,7 @@
 #include sys/utsname.h
 
 #include viralloc.h
+#include virfile.h
 #include virlog.h
 #include virstring.h
 #include cpu/cpu.h
@@ -104,3 +105,39 @@ virBhyveCapsBuild(void)
 virObjectUnref(caps);
 return NULL;
 }
+
+int
+virBhyveProbeGrubCaps(unsigned *caps)
+{
+char *binary, *help;
+virCommandPtr cmd;
+int ret, exit;
+
+ret = 0;
+*caps = 0;
+cmd = NULL;
+help = NULL;
+
+binary = virFindFileInPath(grub-bhyve);
+if (binary == NULL)
+goto out;
+if (!virFileIsExecutable(binary))
+goto out;
+
+cmd = virCommandNew(binary);
+virCommandAddArg(cmd, --help);
+virCommandSetOutputBuffer(cmd, help);
+if (virCommandRun(cmd, exit)  0) {
+ret = -1;
+goto out;
+}
+
+if (strstr(help, --cons-dev) != NULL)
+*caps |= BHYVE_GRUB_CAP_CONSDEV;
+
+ out:
+VIR_FREE(help);
+virCommandFree(cmd);
+VIR_FREE(binary);
+return ret;
+}
diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h
index c52e0d0..f4ebd90 100644
--- a/src/bhyve/bhyve_capabilities.h
+++ b/src/bhyve/bhyve_capabilities.h
@@ -26,4 +26,7 @@
 
 virCapsPtr virBhyveCapsBuild(void);
 
+# define BHYVE_GRUB_CAP_CONSDEV 0x0001
+int virBhyveProbeGrubCaps(unsigned *caps);
+
 #endif
-- 
1.9.3

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

[libvirt] [PATCHv7 1/7] bhyve: Support /domain/bootloader configuration for non-FreeBSD guests.

2014-10-30 Thread Conrad Meyer

We still default to bhyveloader(1) if no explicit bootloader
configuration is supplied in the domain.

If the /domain/bootloader looks like grub-bhyve and the user doesn't
supply /domain/bootloader_args, we make an intelligent guess and try
chainloading the first partition on the disk (or a CD if one exists,
under the assumption that for a VM a CD is likely an install source).

Caveat: Assumes the HDD boots from the msdos1 partition. I think this is
a pretty reasonable assumption for a VM. (DrvBhyve with Bhyveload
already assumes that the first disk should be booted.)

I've tested both HDD and CD boot and they seem to work.
---
 docs/drvbhyve.html.in | 100 +--
 docs/formatdomain.html.in |   4 +-
 src/bhyve/bhyve_command.c | 173 +-
 src/bhyve/bhyve_command.h |   5 +-
 src/bhyve/bhyve_driver.c  |   3 +-
 src/bhyve/bhyve_process.c |  38 +-
 6 files changed, 295 insertions(+), 28 deletions(-)

diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in
index 39afdf5..bd4b35e 100644
--- a/docs/drvbhyve.html.in
+++ b/docs/drvbhyve.html.in
@@ -37,8 +37,7 @@ bhyve+ssh://r...@example.com/system (remote access, SSH 
tunnelled)
 h3Example config/h3
 p
 The bhyve driver in libvirt is in its early stage and under active 
development. So it supports
-only limited number of features bhyve provides. All the supported features 
could be found
-in this sample domain XML.
+only limited number of features bhyve provides.
 /p
 
 p
@@ -48,10 +47,21 @@ disk device were supported per-domain. However,
 up to 31 PCI devices.
 /p
 
+p
+Note: the Bhyve driver in libvirt will boot whichever device is first. If you
+want to install from CD, put the CD device first. If not, put the root HDD
+first.
+/p
+
+p
+Note: Only the SATA bus is supported. Only codecdrom/code- and
+codedisk/code-type disks are supported.
+/p
+
 pre
 lt;domain type='bhyve'gt;
-  lt;namegt;bhyvelt;/namegt;
-  lt;uuidgt;df3be7e7-a104-11e3-aeb0-50e5492bd3dclt;/uuidgt;
+lt;namegt;bhyvelt;/namegt;
+lt;uuidgt;df3be7e7-a104-11e3-aeb0-50e5492bd3dclt;/uuidgt;
 lt;memorygt;219136lt;/memorygt;
 lt;currentMemorygt;219136lt;/currentMemorygt;
 lt;vcpugt;1lt;/vcpugt;
@@ -76,6 +86,7 @@ up to 31 PCI devices.
 lt;driver name='file' type='raw'/gt;
 lt;source file='/path/to/cdrom.iso'/gt;
 lt;target dev='hdc' bus='sata'/gt;
+lt;readonly/gt;
   lt;/diskgt;
   lt;interface type='bridge'gt;
 lt;model type='virtio'/gt;
@@ -85,6 +96,53 @@ up to 31 PCI devices.
 lt;/domaingt;
 /pre
 
+p(The lt;diskgt; sections may be swapped in order to install from
+emcdrom.iso/em.)/p
+
+h3Example config (Linux guest)/h3
+
+p
+Note the addition of lt;bootloadergt;.
+/p
+
+pre
+lt;domain type='bhyve'gt;
+lt;namegt;linux_guestlt;/namegt;
+lt;uuidgt;df3be7e7-a104-11e3-aeb0-50e5492bd3dclt;/uuidgt;
+lt;memorygt;131072lt;/memorygt;
+lt;currentMemorygt;131072lt;/currentMemorygt;
+lt;vcpugt;1lt;/vcpugt;
+lt;bootloadergt;/usr/local/sbin/grub-bhyvelt;/bootloadergt;
+lt;osgt;
+   lt;typegt;hvmlt;/typegt;
+lt;/osgt;
+lt;featuresgt;
+  lt;apic/gt;
+  lt;acpi/gt;
+lt;/featuresgt;
+lt;clock offset='utc'/gt;
+lt;on_poweroffgt;destroylt;/on_poweroffgt;
+lt;on_rebootgt;restartlt;/on_rebootgt;
+lt;on_crashgt;destroylt;/on_crashgt;
+lt;devicesgt;
+  lt;disk type='file' device='disk'gt;
+lt;driver name='file' type='raw'/gt;
+lt;source file='/path/to/guest_hdd.img'/gt;
+lt;target dev='hda' bus='sata'/gt;
+  lt;/diskgt;
+  lt;disk type='file' device='cdrom'gt;
+lt;driver name='file' type='raw'/gt;
+lt;source file='/path/to/cdrom.iso'/gt;
+lt;target dev='hdc' bus='sata'/gt;
+lt;readonly/gt;
+  lt;/diskgt;
+  lt;interface type='bridge'gt;
+lt;model type='virtio'/gt;
+lt;source bridge=virbr0/gt;
+  lt;/interfacegt;
+lt;/devicesgt;
+lt;/domaingt;
+/pre
 
 h2a name=usageGuest usage / management/a/h2
 
@@ -119,6 +177,20 @@ to let a guest boot or start a guest using:/p
 
 prestart --console domname/pre
 
+pbNB:/b An bootloader configured to require user interaction will prevent
+the domain from starting (and thus codevirsh console/code or codestart
+--console/code from functioning) until the user interacts with it manually on
+the VM host. Because users typically do not have access to the VM host,
+interactive bootloaders are unsupported by libvirt. emHowever,/em if you 
happen to
+run into this scenario and also happen to have access to the Bhyve host
+machine, you may select a boot option and allow the domain to finish starting
+by using an alternative terminal client on the VM host to connect to the
+domain-configured null modem device. One example (assuming
+code/dev/nmdm0B/code is configured as the slave end of the domain serial
+device) is:/p
+
+precu -l /dev/nmdm0B/pre
+
 h3a name=xmltonativeConverting from domain

[libvirt] [PATCHv7 0/7] Add non-FreeBSD guest support to Bhyve driver.

2014-10-30 Thread Conrad Meyer

Drvbhyve hardcodes bhyveload(8) as the host bootloader for guests.
bhyveload(8) loader only supports FreeBSD guests.

This patch series adds bootloader and bootloader_args handling to
bhyve_command, so libvirt can boot non-FreeBSD guests in Bhyve.

Additionally, support for grub-bhyve(1)'s --cons-dev argument is added so that
interactive GRUB menus can be manipulated with the domain-configured serial
device.

See patch logs for further details.

Thanks,
Conrad

Changes in v7:
  - Detect grub-bhyve support for --cons-dev automatically; only pass the
domain-configured serial device if supported.
  - Add '-nocons' version of the grub-serial unit tests to confirm we construct
the loader Cmd correctly if the capability is missing.


Conrad Meyer (7):
  bhyve: Support /domain/bootloader configuration for non-FreeBSD
guests.
  bhyvexml2argv: Add loader argv tests.
  domaincommon.rng: Add 'bootloader' to os=hvm schema for Bhyve
  bhyvexml2argv: Add tests for domain-configured bootloader, args
  bhyve: Probe grub-bhyve for --cons-dev capability
  bhyve: Add console support for grub-bhyve bootloader
  bhyvexml2argv: Add test for grub console support

 docs/drvbhyve.html.in  | 100 ++-
 docs/formatdomain.html.in  |   4 +-
 docs/schemas/domaincommon.rng  |  17 +-
 src/bhyve/bhyve_capabilities.c |  37 
 src/bhyve/bhyve_capabilities.h |   3 +
 src/bhyve/bhyve_command.c  | 189 +++--
 src/bhyve/bhyve_command.h  |   5 +-
 src/bhyve/bhyve_driver.c   |  16 +-
 src/bhyve/bhyve_driver.h   |   2 +
 src/bhyve/bhyve_process.c  |  38 -
 src/bhyve/bhyve_utils.h|   2 +
 .../bhyvexml2argv-acpiapic.ldargs  |   1 +
 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs  |   1 +
 .../bhyvexml2argv-bhyveload-explicitargs.args  |   3 +
 .../bhyvexml2argv-bhyveload-explicitargs.ldargs|   1 +
 .../bhyvexml2argv-bhyveload-explicitargs.xml   |  23 +++
 .../bhyvexml2argvdata/bhyvexml2argv-console.ldargs |   1 +
 .../bhyvexml2argv-custom-loader.args   |   3 +
 .../bhyvexml2argv-custom-loader.ldargs |   1 +
 .../bhyvexml2argv-custom-loader.xml|  24 +++
 .../bhyvexml2argv-disk-cdrom-grub.args |   3 +
 .../bhyvexml2argv-disk-cdrom-grub.devmap   |   1 +
 .../bhyvexml2argv-disk-cdrom-grub.ldargs   |   2 +
 .../bhyvexml2argv-disk-cdrom-grub.xml  |  23 +++
 .../bhyvexml2argv-disk-cdrom.ldargs|   1 +
 .../bhyvexml2argv-disk-virtio.ldargs   |   1 +
 .../bhyvexml2argv-grub-defaults.args   |   3 +
 .../bhyvexml2argv-grub-defaults.devmap |   1 +
 .../bhyvexml2argv-grub-defaults.ldargs |   2 +
 .../bhyvexml2argv-grub-defaults.xml|  23 +++
 .../bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs |   1 +
 .../bhyvexml2argv-serial-grub-nocons.args  |   4 +
 .../bhyvexml2argv-serial-grub-nocons.devmap|   1 +
 .../bhyvexml2argv-serial-grub-nocons.ldargs|   2 +
 .../bhyvexml2argv-serial-grub-nocons.xml   |  26 +++
 .../bhyvexml2argv-serial-grub.args |   4 +
 .../bhyvexml2argv-serial-grub.devmap   |   1 +
 .../bhyvexml2argv-serial-grub.ldargs   |   2 +
 .../bhyvexml2argv-serial-grub.xml  |  26 +++
 .../bhyvexml2argvdata/bhyvexml2argv-serial.ldargs  |   1 +
 tests/bhyvexml2argvtest.c  |  71 +++-
 41 files changed, 631 insertions(+), 39 deletions(-)
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs
 create mode 100644

[libvirt] [PATCHv7 4/7] bhyvexml2argv: Add tests for domain-configured bootloader, args

2014-10-30 Thread Conrad Meyer

---
 .../bhyvexml2argv-bhyveload-explicitargs.args  |  3 +++
 .../bhyvexml2argv-bhyveload-explicitargs.ldargs|  1 +
 .../bhyvexml2argv-bhyveload-explicitargs.xml   | 23 +
 .../bhyvexml2argv-custom-loader.args   |  3 +++
 .../bhyvexml2argv-custom-loader.ldargs |  1 +
 .../bhyvexml2argv-custom-loader.xml| 24 ++
 .../bhyvexml2argv-disk-cdrom-grub.args |  3 +++
 .../bhyvexml2argv-disk-cdrom-grub.devmap   |  1 +
 .../bhyvexml2argv-disk-cdrom-grub.ldargs   |  2 ++
 .../bhyvexml2argv-disk-cdrom-grub.xml  | 23 +
 .../bhyvexml2argv-grub-defaults.args   |  3 +++
 .../bhyvexml2argv-grub-defaults.devmap |  1 +
 .../bhyvexml2argv-grub-defaults.ldargs |  2 ++
 .../bhyvexml2argv-grub-defaults.xml| 23 +
 tests/bhyvexml2argvtest.c  |  4 
 15 files changed, 117 insertions(+)
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.devmap
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml

diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args
new file mode 100644
index 000..4122e62
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args
@@ -0,0 +1,3 @@
+/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \
+-s 2:0,ahci-hd,/tmp/freebsd.img bhyve
diff --git 
a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs
new file mode 100644
index 000..a3f58f0
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -X -Y -Z
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml
new file mode 100644
index 000..65823cd
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml
@@ -0,0 +1,23 @@
+domain type='bhyve'
+  namebhyve/name
+  uuiddf3be7e7-a104-11e3-aeb0-50e5492bd3dc/uuid
+  memory219136/memory
+  vcpu1/vcpu
+  bootloader_args-X -Y -Z/bootloader_args
+  os
+typehvm/type
+  /os
+  devices
+disk type='file'
+  driver name='file' type='raw'/
+  source file='/tmp/freebsd.img'/
+  target dev='hda' bus='sata'/
+  address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
+/disk
+interface type='bridge'
+  model type='virtio'/
+  source bridge=virbr0/
+  address type='pci' domain='0x' bus='0x00' slot='0x03' 
function='0x0'/
+/interface
+  /devices
+/domain
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args
new file mode 100644
index 000..4122e62
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args
@@ -0,0 +1,3 @@
+/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \
+-s 2:0,ahci-hd,/tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs
new file mode 100644
index 000..100e163
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs
@@ -0,0 +1 @@
+/fizz_buzz_bazz -X -Y -Z
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml
new file mode 100644
index 000..e9e0d5e
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml
@@ -0,0 +1,24 @@
+domain type='bhyve'
+  namebhyve/name
+  uuiddf3be7e7-a104-11e3-aeb0-50e5492bd3dc/uuid
+  memory219136/memory
+  vcpu1/vcpu
+  bootloader/fizz_buzz_bazz/bootloader
+

[libvirt] [PATCHv7 2/7] bhyvexml2argv: Add loader argv tests.

2014-10-30 Thread Conrad Meyer

---
 .../bhyvexml2argv-acpiapic.ldargs  |  1 +
 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs  |  1 +
 .../bhyvexml2argvdata/bhyvexml2argv-console.ldargs |  1 +
 .../bhyvexml2argv-disk-cdrom.ldargs|  1 +
 .../bhyvexml2argv-disk-virtio.ldargs   |  1 +
 .../bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs |  1 +
 .../bhyvexml2argvdata/bhyvexml2argv-serial.ldargs  |  1 +
 tests/bhyvexml2argvtest.c  | 60 +++---
 8 files changed, 61 insertions(+), 6 deletions(-)
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs

diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs
new file mode 100644
index 000..215d65f
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs
new file mode 100644
index 000..215d65f
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs
new file mode 100644
index 000..215d65f
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs
new file mode 100644
index 000..0eb3cc9
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -m 214 -d /tmp/cdrom.iso bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs
new file mode 100644
index 000..215d65f
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs
new file mode 100644
index 000..215d65f
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs
new file mode 100644
index 000..215d65f
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs
@@ -0,0 +1 @@
+/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index b9be378..3ff6696 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -15,14 +15,16 @@
 static bhyveConn driver;
 
 static int testCompareXMLToArgvFiles(const char *xml,
- const char *cmdline)
+ const char *cmdline,
+ const char *ldcmdline,
+ const char *dmcmdline)
 {
-char *expectargv = NULL;
+char *expectargv = NULL, *expectld = NULL, *expectdm = NULL;
 int len;
-char *actualargv = NULL;
+char *actualargv = NULL, *actualld = NULL, *actualdm = NULL;
 virDomainDefPtr vmdef = NULL;
 virDomainObj vm;
-virCommandPtr cmd = NULL;
+virCommandPtr cmd = NULL, ldcmd = NULL;
 virConnectPtr conn;
 int ret = -1;
 
@@ -42,6 +44,16 @@ static int testCompareXMLToArgvFiles(const char *xml,
 if (!(actualargv = virCommandToString(cmd)))
 goto out;
 
+if (!(ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, device.map,
+  actualdm)))
+goto out;
+
+if (actualdm != NULL)
+virTrimSpaces(actualdm, NULL);
+
+if (!(actualld = virCommandToString(ldcmd)))
+goto out;
+
 len = virtTestLoadFile(cmdline, expectargv);
 if (len  0)
 goto out;
@@ -49,17 +61,49 @@ static int testCompareXMLToArgvFiles(const char *xml,
 if (len  expectargv[len - 1] == '\n')
 expectargv[len - 1] = '\0';
 
+len = virtTestLoadFile(ldcmdline, expectld);
+if (len  0)
+goto out;
+
+if (len  expectld[len - 1] == '\n')
+expectld[len - 1] = '\0';
+
+len =

[libvirt] [PATCHv7 7/7] bhyvexml2argv: Add test for grub console support

2014-10-30 Thread Conrad Meyer

---
 .../bhyvexml2argv-serial-grub-nocons.args  |  4 
 .../bhyvexml2argv-serial-grub-nocons.devmap|  1 +
 .../bhyvexml2argv-serial-grub-nocons.ldargs|  2 ++
 .../bhyvexml2argv-serial-grub-nocons.xml   | 26 ++
 .../bhyvexml2argv-serial-grub.args |  4 
 .../bhyvexml2argv-serial-grub.devmap   |  1 +
 .../bhyvexml2argv-serial-grub.ldargs   |  2 ++
 .../bhyvexml2argv-serial-grub.xml  | 26 ++
 tests/bhyvexml2argvtest.c  |  7 ++
 9 files changed, 73 insertions(+)
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml

diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args
new file mode 100644
index 000..df50290
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args
@@ -0,0 +1,4 @@
+/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \
+-s 2:0,ahci-hd,/tmp/freebsd.img \
+-s 1,lpc -l com1,/dev/nmdm0A bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap
new file mode 100644
index 000..68c35da
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap
@@ -0,0 +1 @@
+(hd0) /tmp/freebsd.img
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs
new file mode 100644
index 000..91c15ce
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs
@@ -0,0 +1,2 @@
+/usr/local/sbin/grub-bhyve --root hd0,msdos1 --device-map 'device.map' \
+--memory 214 bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml
new file mode 100644
index 000..8c451f7
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml
@@ -0,0 +1,26 @@
+domain type='bhyve'
+  namebhyve/name
+  uuiddf3be7e7-a104-11e3-aeb0-50e5492bd3dc/uuid
+  memory219136/memory
+  vcpu1/vcpu
+  bootloader/usr/local/sbin/grub-bhyve/bootloader
+  os
+typehvm/type
+  /os
+  devices
+disk type='file'
+  driver name='file' type='raw'/
+  source file='/tmp/freebsd.img'/
+  target dev='hda' bus='sata'/
+  address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
+/disk
+interface type='bridge'
+  model type='virtio'/
+  source bridge=virbr0/
+  address type='pci' domain='0x' bus='0x00' slot='0x03' 
function='0x0'/
+/interface
+serial type='nmdm'
+  source master='/dev/nmdm0A' slave='/dev/nmdm0B'/
+/serial
+  /devices
+/domain
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args
new file mode 100644
index 000..df50290
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args
@@ -0,0 +1,4 @@
+/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \
+-s 2:0,ahci-hd,/tmp/freebsd.img \
+-s 1,lpc -l com1,/dev/nmdm0A bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap
new file mode 100644
index 000..68c35da
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap
@@ -0,0 +1 @@
+(hd0) /tmp/freebsd.img
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs
new file mode 100644
index 000..5645097
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs
@@ -0,0 +1,2 @@
+/usr/local/sbin/grub-bhyve --root hd0,msdos1 --device-map 'device.map' \
+--memory 214 --cons-dev /dev/nmdm0A bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml
new file mode 100644
index 000..8c451f7
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml
@@ -0,0 +1,26 @@
+domain type='bhyve'
+  namebhyve/name
+  uuiddf3be7e7-a104-11e3-aeb0-50e5492bd3dc/uuid
+  memory219136/memory
+  vcpu1/vcpu
+  bootloader/usr/local/sbin/grub-bhyve/bootloader
+  os
+

Re: [libvirt] [PATCH v2 2/2] qemu: make sure capability probing process can start

2014-10-30 Thread Martin Kletzander


On Thu, Oct 30, 2014 at 10:42:05AM +0100, Peter Krempa wrote:

On 10/12/14 14:12, Martin Kletzander wrote:

When daemon is killed right in the middle of probing a qemu binary for
its capabilities, the VM is left running.  Next time the daemon is


s/VM/qemu process/ ? The qemu isn't running anything so it might confuse
someone.


starting, it cannot start qemu process because the one that's already
running does have the pidfile flock()'d.

Reported-by: Wang Yufei james.wangyu...@huawei.com

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
 - Don't use /proc, but simply just try to acquire the pidfile.
 - https://www.redhat.com/archives/libvir-list/2014-October/msg00320.html

 src/qemu/qemu_capabilities.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6fcb5c7..8aedf3f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3243,6 +3243,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
 config.data.nix.path = monpath;
 config.data.nix.listen = false;

+virPidFileForceCleanupPath(pidfile);
+
 VIR_DEBUG(Try to get caps via QMP qemuCaps=%p, qemuCaps);

 /*


ACK. It is a bugfix, but the code path is is critical. I'm leaning
towards pushing this before the freeze but I'd feel better with yet
another opinion.



I fixed both things you've pointed out and I'll leave the push after
release since I already broke the build once :)

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] conf: use proper maximum for parsing memory values

2014-10-30 Thread Eric Blake

On 10/30/2014 09:55 AM, Eric Blake wrote:
 On 10/30/2014 09:49 AM, Martin Kletzander wrote:
 The value is stored in unsigned long long, so ULLONG_MAX is the proper
 upper limit to use.
 
 No, it's not.
 

 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
 Even though this is a build-breaker (for 32bit systems
 memtune-unlimited fails to parse), I'm not pushing it as one because
 it feels odd that such change wouldn't break anything else.
 
 What's the compile failure?  This patch is intentionally trying to fit
 the largest value that will fit in an unsigned long when scaled by
 kilobytes, while still parsing by bytes.  Thus, on 64-bit machines, you
 can parse 0x bytes, then scale down by 1024 and store in
 unsigned long; on 32-bit machines, your maximum has to be limited to
 0x*1024 bytes before scaling back down.

Sounds like the real bug is in the memtune-unlimited test.  2^53-1 is
the maximum memory for a 64-bit host, but invalid on a 32-bit host.  Any
interface that uses 'unsigned long' as its capping point has to have
different maximum limits on 32-bit hosts.

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



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

[libvirt] [PATCH] Spell TIOCSCTTY right in the error message

2014-10-30 Thread Ján Tomko

---
 src/lxc/lxc_container.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Pushed as trivial.

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 2af2674..f02b959 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -281,7 +281,7 @@ static int lxcContainerSetupFDs(int *ttyfd,
 
 if (ioctl(*ttyfd, TIOCSCTTY, NULL)  0) {
 virReportSystemError(errno, %s,
- _(ioctl(TIOCSTTY) failed));
+ _(ioctl(TIOCSCTTY) failed));
 goto cleanup;
 }
 
-- 
2.0.4

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

Re: [libvirt] [PATCH] conf: use proper maximum for parsing memory values

2014-10-30 Thread Eric Blake

On 10/30/2014 10:09 AM, Eric Blake wrote:
 On 10/30/2014 09:55 AM, Eric Blake wrote:
 On 10/30/2014 09:49 AM, Martin Kletzander wrote:
 The value is stored in unsigned long long, so ULLONG_MAX is the proper
 upper limit to use.

 No, it's not.


 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
 Even though this is a build-breaker (for 32bit systems
 memtune-unlimited fails to parse), I'm not pushing it as one because
 it feels odd that such change wouldn't break anything else.

 What's the compile failure?  This patch is intentionally trying to fit
 the largest value that will fit in an unsigned long when scaled by
 kilobytes, while still parsing by bytes.  Thus, on 64-bit machines, you
 can parse 0x bytes, then scale down by 1024 and store in
 unsigned long; on 32-bit machines, your maximum has to be limited to
 0x*1024 bytes before scaling back down.
 
 Sounds like the real bug is in the memtune-unlimited test.  2^53-1 is
 the maximum memory for a 64-bit host, but invalid on a 32-bit host.  Any
 interface that uses 'unsigned long' as its capping point has to have
 different maximum limits on 32-bit hosts.

Or maybe the problem is that at some point we used unsigned long, and
later moved to unsigned long long, but never updated the comment?  I'm
trying to investigate the history of this code...

-- 
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 0/2] libxl: user-specified domain config improvements

2014-10-30 Thread Jim Fehlig

Jim Fehlig wrote:
 Michal Privoznik wrote:
   
 On 11.10.2014 00:03, Jim Fehlig wrote:
 
 V2 of a small series to improve libxl driver's support for
 user-specified
 config

 https://www.redhat.com/archives/libvir-list/2014-September/msg01243.html

 Patches 1-4 of that series have been pushed.  In patch 5, Michal
 suggested checking that the user-specified emulator is executable.
 Patch 1 of this series is a respin of patch 5 that includes Michal's
 suggestion.  Patch 2 fixes an issue found while testing.

 Jim Fehlig (2):
libxl: Support user-specified emulator
libxl: fix double-free of libxl_domain_build_info

   src/libxl/libxl_conf.c | 48
 +---
   1 file changed, 33 insertions(+), 15 deletions(-)

   
 ACK
 

 Is it ok to push these during 1.2.10 freeze?  2/2 is a bug fix...
   

Pushed these after confirmation from Laine on IRC.

Regards,
Jim

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

Re: [libvirt] ceps-based backing stores

2014-10-30 Thread Lyssa P. Livingston

Thanks.  I’ll be looking for the patch.

Lyssa

 On Oct 30, 2014, at 03:03, Peter Krempa pkre...@redhat.com wrote:
 
 On 10/30/14 01:15, Lyssa P. Livingston wrote:
 I have a file with an ceph-based backing store.  I run this command:
 
nova boot vm-clone
 
 where vm-clone is the rbd-backed file.  It generates this command:
 
qemu-img create -f qcow2 -b 
 rbd:rbdpool/disk/instance-0002.0.disk:conf=/etc/ceph/ceph.conf:id=admin 
 -F raw /mnt/novadisk/nova/instances/3efa8a7b-75a8-4695-8fdd-659185c46b7d/disk
 
 and I am getting this error:
 
error: internal error: backing store parser is not implemented for 
 protocol rbd
 
 Libvirt is parsing the information saved in the backing store field of
 an image to re-create the backing chain structure in memory.
 Unfortunately I didn't implement all the possible formats for this as
 there's a lot of them and one of the missing pieces is the colon syntax
 for RBD volumes.
 
 I'll try to come up with a patch for this that will allow to recognize
 the RBD volume as a remote one where libvirt isn't able to access it and
 allow to start the VM.
 
 
 I was under the impression that qemu could work with ceph-based backing 
 stores, so is there something I should run first, or files that I should 
 patch, or ... ???  I am running libvirt 1.2.9.  Thanks for any assistance 
 you can provide.
 
 Qemu is able to handle those. Libvirt currently doesn't support the syntax.
 
 
  Lyssa
 
 
 
 Peter
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list
 
 
 


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

Re: [libvirt] [PATCH] conf: use proper maximum for parsing memory values

2014-10-30 Thread Eric Blake

On 10/30/2014 10:23 AM, Eric Blake wrote:
 
 Or maybe the problem is that at some point we used unsigned long, and
 later moved to unsigned long long, but never updated the comment?  I'm
 trying to investigate the history of this code...

Looking a bit deeper, commit 4888f0fb5 was where we changed all internal
tracking to use unsigned long long; but we still have the public API
virDomain{Get,Set}MaxMemory that are tied to 'unsigned long'.  I guess
what I was trying to do in 2e22f23 (both in 2012) was to ensure that
virDomainGetMaxMemory would never have to fail due to overflow, by
capping the input to something that would fit in the output.  And we
still got it wrong then, since commit 19e7c04 (in 2013) fixed a case
where 32-bit hosts were truncating incorrectly.

Meanwhile, it looks like virDomain{Get,Set}MemoryParamters have ALWAYS
used ullong.  But this interface doesn't report max memory - so we still
have to live with the fact that there is currently no interface for the
user getting at max memory other than crippled API.

Maybe the solution is to enhance virDomain{Get,Set}MemoryParameters to
be a superset of the older APIs of virDomain{Get,Set}MaxMemory, and
teach the older APIs to fail with VIR_ERR_OVERFLOW when the value can
only be passed through the newer API.

Or maybe the solution is to make virDomainParseMemory add a bool
parameter that says whether the caller is old-style (32-bit cap) or
new-style (full 64-bit width), and set the maximum according to that
information.  That's probably the easiest for doing right now.  I can
take a stab at it, since it was my commit in 2012 that originally
introduced the weird 32-bit cap even when parsing a 64-bit field.

-- 
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 0/2] Improve error messages about blkio values

2014-10-30 Thread Michal Privoznik


On 29.10.2014 16:44, Martin Kletzander wrote:

There was one message used for all parsing errors in the blkio code.
This series fixes it in both qemu and lxc drivers where the code is
used.

Martin Kletzander (2):
   qemu: improve error message for invalid blkiotune settings
   lxc: improve error message for invalid blkiotune settings

  src/lxc/lxc_driver.c   | 29 +++--
  src/qemu/qemu_driver.c | 29 +++--
  2 files changed, 38 insertions(+), 20 deletions(-)



ACK and safe for the freeze.

Michal

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

Re: [libvirt] [PATCH] qemu: make advice from numad available when building commandline

2014-10-30 Thread Michal Privoznik


On 30.10.2014 07:40, Martin Kletzander wrote:

Particularly in qemuBuildNumaArgStr(), there was a need for the advice
due to memory backing, which needs to know the nodeset it will be pinned
to.  With newer qemu this caused the following error when starting
domain:

   error: internal error: Advice from numad is needed in case of
   automatic numa placement

even when starting perfectly valid domain, e.g.:

   ...
   vcpu placement='auto'4/vcpu
   numatune
 memory mode='strict' placement='auto'/
   /numatune
   cpu
 numa
   cell id='0' cpus='0' memory='524288'/
   cell id='1' cpus='1' memory='524288'/
 /numa
   /cpu
   ...

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

Signed-off-by: Martin Kletzander mklet...@redhat.com


Would it be possible to add a test case? Maybe you'd need to mock some 
functions but we already have qemuxml2argvmock.c. Otherwise the code 
looks okay and with test case I'd ACK it for the freeze.


Michal

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

Re: [libvirt] [PATCH 3/3] Avoid rare race when undefining domain

2014-10-30 Thread Michal Privoznik


On 30.10.2014 16:04, Martin Kletzander wrote:

When one domain is being undefined and at the same time started, for
example, there is a possibility of a rare problem occuring.

  - Thread 1 does virDomainUndefine(), has the lock, checks that the
domain is active and because it's not, calls
virDomainObjListRemove().

  - Thread 2 does virDomainCreate() and tries to lock the domain.

  - Thread 1 needs to lock domain list in order to remove the domain from
it, but must unlock domain first (proper order is to lock domain list
first and the domain itself second).

  - Thread 2 grabs the lock, starts the domain and releases the lock.

  - Thread 1 grabs the lock and removes the domain from list.

With this patch:

  - The undefining domain gets marked as to undefine before it is
unlocked.

  - If domain is found in any of the search APIs, it's returned only if
it is not marked as to undefine.  The check is done while the
domain is locked.

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

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
  src/conf/domain_conf.c | 23 ---
  src/conf/domain_conf.h |  1 +
  2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 43574e1..b92a58a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1054,8 +1054,13 @@ virDomainObjPtr 
virDomainObjListFindByID(virDomainObjListPtr doms,
  virDomainObjPtr obj;
  virObjectLock(doms);
  obj = virHashSearch(doms-objs, virDomainObjListSearchID, id);
-if (obj)
+if (obj) {
  virObjectLock(obj);
+if (obj-undefining) {
+virObjectUnlock(obj);
+obj = NULL;
+}
+}
  virObjectUnlock(doms);
  return obj;
  }


I find this too hackish, Wouldn't it be better to hold the domain list 
locked during the whole undefine procedure? On one hand the throughput 
would get hurt but on the other, the code would be cleaner. Or maybe we 
can just make the Undefine() API to grab the QEMU_JOB_MODIFY job which 
would make the APIs serialize.


Michal

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

Re: [libvirt] [PATCH 1/3] virsh: don't list unknown domains

2014-10-30 Thread Michal Privoznik


On 30.10.2014 16:04, Martin Kletzander wrote:

When the list of domains is fetched and being printed, but in the
meantime one domain was undefined before its status was fetched, the
output then includes domain with no state.  With this patch, such
domain is skipped over as consecutive 'virsh list --all' (or the same
one ran a second later) wouldn't list it anyway.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
  tools/virsh-domain-monitor.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 2af0d4f..4e434f8 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1916,6 +1916,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
  ignore_value(virStrcpyStatic(id_buf, -));

  state = vshDomainState(ctl, dom, NULL);
+
+/* Domain could've been removed in the meantime */
+if (state  0)
+continue;
+
  if (optTable  managed  state == VIR_DOMAIN_SHUTOFF 
  virDomainHasManagedSaveImage(dom, 0)  0)
  state = -2;



ACK and safe for freeze. This may happen esp. when using the old, 
non-atomic APIs to list domains. For instance, domain is shutoff, then 
undefine happens and then we query the domain state.


Michal

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

Re: [libvirt] [PATCH 3/3] Avoid rare race when undefining domain

2014-10-30 Thread Daniel P. Berrange

On Thu, Oct 30, 2014 at 06:39:56PM +0100, Michal Privoznik wrote:
 On 30.10.2014 16:04, Martin Kletzander wrote:
 When one domain is being undefined and at the same time started, for
 example, there is a possibility of a rare problem occuring.
 
   - Thread 1 does virDomainUndefine(), has the lock, checks that the
 domain is active and because it's not, calls
 virDomainObjListRemove().
 
   - Thread 2 does virDomainCreate() and tries to lock the domain.
 
   - Thread 1 needs to lock domain list in order to remove the domain from
 it, but must unlock domain first (proper order is to lock domain list
 first and the domain itself second).
 
   - Thread 2 grabs the lock, starts the domain and releases the lock.
 
   - Thread 1 grabs the lock and removes the domain from list.
 
 With this patch:
 
   - The undefining domain gets marked as to undefine before it is
 unlocked.
 
   - If domain is found in any of the search APIs, it's returned only if
 it is not marked as to undefine.  The check is done while the
 domain is locked.
 
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150505
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
   src/conf/domain_conf.c | 23 ---
   src/conf/domain_conf.h |  1 +
   2 files changed, 21 insertions(+), 3 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 43574e1..b92a58a 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -1054,8 +1054,13 @@ virDomainObjPtr 
 virDomainObjListFindByID(virDomainObjListPtr doms,
   virDomainObjPtr obj;
   virObjectLock(doms);
   obj = virHashSearch(doms-objs, virDomainObjListSearchID, id);
 -if (obj)
 +if (obj) {
   virObjectLock(obj);
 +if (obj-undefining) {
 +virObjectUnlock(obj);
 +obj = NULL;
 +}
 +}
   virObjectUnlock(doms);
   return obj;
   }
 
 I find this too hackish, Wouldn't it be better to hold the domain list
 locked during the whole undefine procedure? On one hand the throughput would
 get hurt but on the other, the code would be cleaner. Or maybe we can just
 make the Undefine() API to grab the QEMU_JOB_MODIFY job which would make the
 APIs serialize.

Yep, it feels like Undefine could be treated as a job, so that all
the other APIs get blocked/serialized by it.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

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

2014-10-30 Thread Michal Privoznik


On 30.10.2014 16:14, 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
(excluding NUMA!) 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  |  6 --
  2 files changed, 17 deletions(-)


Looking at the commit you are referring to, I spot other interesting 
things too. For instance, you are introducing @cfg variable to 
qemuDomainGetNumaParameters() but it's not used anywhere in the function 
(besides getting and unrefing driver's config) which is weird. So while 
you are at revert, you may revert that part too.




diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..be8e335 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7835,17 +7835,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 373daab..631cb5f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10437,12 +10437,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;




I've always felt we should use a different approach. If we want to check 
the permissions we should do that as close to the actual place needing 
root permissions as possible. With the complexity of call tree in 
libvirt it's easy to forget about one possible path. I mean, I like the 
approach 2/2 more then this old one.


Michal

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

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

2014-10-30 Thread Michal Privoznik


On 30.10.2014 16:14, Erik Skultety wrote:

Patch 43b67f2e disallowed network tuning only with qemu driver, however
this patch moved the check for root privileges into
virNetDevBandwidthSetInternal wrapper function, so the call should now
fail in all possible cases. The reason to create another wrapper is that
we do execute a test suite with user privileges during compilation.
Tests fail unless the wrapper is used.
---
  src/libvirt_private.syms  |  4 
  src/util/virnetdevbandwidth.c | 31 ---
  src/util/virnetdevbandwidthpriv.h | 36 
  tests/virnetdevbandwidthtest.c|  5 +++--
  4 files changed, 67 insertions(+), 9 deletions(-)
  create mode 100644 src/util/virnetdevbandwidthpriv.h

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9d5c814..ab3d6d0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1647,6 +1647,10 @@ virNetDevBandwidthUnplug;
  virNetDevBandwidthUpdateRate;


+# util/virnetdevbandwidthpriv.h
+virNetDevBandwidthSetInternal;
+
+
  # util/virnetdevbridge.h
  virNetDevBridgeAddPort;
  virNetDevBridgeCreate;
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 5fa231a..1fc6fdf 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -21,8 +21,9 @@
   */

  #include config.h
-
-#include virnetdevbandwidth.h
+#include unistd.h
+#define __VIR_BANDWIDTH_PRIV_H_ALLOW__
+#include virnetdevbandwidthpriv.h
  #include vircommand.h
  #include viralloc.h
  #include virerror.h
@@ -41,9 +42,8 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
  VIR_FREE(def);
  }

-
  /**
- * virNetDevBandwidthSet:
+ * virNetDevBandwidthSetInternal:
   * @ifname: on which interface
   * @bandwidth: rates to set (may be NULL)
   * @hierarchical_class: whether to create hierarchical class
@@ -58,9 +58,9 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
   * Return 0 on success, -1 otherwise.
   */
  int
-virNetDevBandwidthSet(const char *ifname,
-  virNetDevBandwidthPtr bandwidth,
-  bool hierarchical_class)
+virNetDevBandwidthSetInternal(const char *ifname,
+  virNetDevBandwidthPtr bandwidth,
+  bool hierarchical_class)
  {
  int ret = -1;
  virCommandPtr cmd = NULL;
@@ -242,6 +242,23 @@ virNetDevBandwidthSet(const char *ifname,
  return ret;
  }

+int
+virNetDevBandwidthSet(const char *ifname,
+  virNetDevBandwidthPtr bandwidth,
+  bool hierarchical_class)
+{
+if (geteuid() != 0) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(Network bandwidth tuning is not available
+  in session mode));
+return -1;
+} else {
+return virNetDevBandwidthSetInternal(ifname,
+ bandwidth,
+ hierarchical_class);
+}
+}
+
  /**
   * virNetDevBandwidthClear:
   * @ifname: on which interface
diff --git a/src/util/virnetdevbandwidthpriv.h 
b/src/util/virnetdevbandwidthpriv.h
new file mode 100644
index 000..9c3a0fa
--- /dev/null
+++ b/src/util/virnetdevbandwidthpriv.h
@@ -0,0 +1,36 @@
+/*
+ * virnetdevbandwidthpriv.h: Functions for testing virNetDevBandwidth APIs
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ */
+
+#ifndef __VIR_BANDWIDTH_PRIV_H_ALLOW__
+# error virnetdevbandwidthpriv.h may only be included by virnetdevbandwidth.c or 
test suites
+#endif
+
+#ifndef __VIR_NETDEVBANDWIDTH_PRIV_H__
+# define __VIR_NETDEVBANDWIDTH_PRIV_H__
+
+# include virnetdevbandwidth.h
+
+int virNetDevBandwidthSetInternal(const char *ifname,
+  virNetDevBandwidthPtr bandwidth,
+  bool hierarchical_class)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
+#endif /* __VIR_NETDEVBANDWIDTH_PRIV_H__ */
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 384991e..a93d0ae 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -23,7 +23,8 @@
  #include testutils.h
  #define __VIR_COMMAND_PRIV_H_ALLOW__
  #include vircommandpriv.h
-#include virnetdevbandwidth.h

Re: [libvirt] [PATCH] conf: use proper maximum for parsing memory values

2014-10-30 Thread Eric Blake

On 10/30/2014 10:51 AM, Eric Blake wrote:
 On 10/30/2014 10:23 AM, Eric Blake wrote:

 Or maybe the problem is that at some point we used unsigned long, and
 later moved to unsigned long long, but never updated the comment?  I'm
 trying to investigate the history of this code...
 

 
 Or maybe the solution is to make virDomainParseMemory add a bool
 parameter that says whether the caller is old-style (32-bit cap) or
 new-style (full 64-bit width), and set the maximum according to that
 information.  That's probably the easiest for doing right now.  I can
 take a stab at it, since it was my commit in 2012 that originally
 introduced the weird 32-bit cap even when parsing a 64-bit field.

I went with this idea:

https://www.redhat.com/archives/libvir-list/2014-October/msg01084.html

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



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

[libvirt] [PATCH v2] domain: fix parsing of memory tunables on 32-bit machines

2014-10-30 Thread Eric Blake

Commit 6c9a8a4 (Oct 2014) exposed a long-standing issue on 32-bit
machines: code related to virDomainSetMemoryParameters has always
been documented as using a 64-bit limit, but it was implemented by
calling virDomainParseMemory which enforced an 'unsigned long'
limit.  Since VIR_DOMAIN_MEMORY_PARAM_UNLIMITED capped to a
long is -1, but virDomainParseScaledValue no longer accepts
negative values, an attempt to use 2^53-1 as a hard memory limit
started failing the testsuite.  However, the problem with capping
things artificially low has existed for much longer - ever since
commits 4888f0fb and 2e22f23 (Mar 2012) switched internal tracking
from 'unsigned long' to 'unsigned long long' (prior to that time,
the cap was a side-effect of the choice of types).  We _have_ to
cap the balloon memory values, (no thanks to baked in 'unsigned long'
of API such as virDomainSetMaxMemory or virDomainGetInfo with no
counterpart API that guarantees 64-bit access to those numbers)
but memory parameters have never needed the artificial limit.

At any rate, the solution is to make the parser function gain a
parameter, and only do the reduced 32-bit cap for the values that
are constrained due to API.

* src/conf/domain_conf.h (_virDomainMemtune): Add comments.
* src/conf/domain_conf.c (virDomainParseMemory): Add parameter.
(virDomainDefParseXML): Adjust callers.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/conf/domain_conf.c | 25 ++---
 src/conf/domain_conf.h | 14 --
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 39befb0..c594325 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6373,19 +6373,22 @@ virDomainParseScaledValue(const char *xpath,


 /* Parse a memory element located at XPATH within CTXT, and store the
- * result into MEM.  If REQUIRED, then the value must exist;
- * otherwise, the value is optional.  The value is in blocks of 1024.
- * Return 0 on success, -1 on failure after issuing error.  */
+ * result into MEM, in blocks of 1024.  If REQUIRED, then the value
+ * must exist; otherwise, the value is optional.  The value must not
+ * exceed VIR_DOMAIN_MEMORY_PARAM_UNLIMITED once scaled; additionally,
+ * if CAPPED is true, the value must fit within an unsigned long (only
+ * matters on 32-bit platforms).  Return 0 on success, -1 on failure
+ * after issuing error.  */
 static int
 virDomainParseMemory(const char *xpath, xmlXPathContextPtr ctxt,
- unsigned long long *mem, bool required)
+ unsigned long long *mem, bool required, bool capped)
 {
 int ret = -1;
 unsigned long long bytes, max;

 /* 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))
+if (capped  sizeof(unsigned long)  sizeof(long long))
 max = 1024ull * ULONG_MAX;
 else
 max = LLONG_MAX;
@@ -12215,11 +12218,11 @@ virDomainDefParseXML(xmlDocPtr xml,

 /* Extract domain memory */
 if (virDomainParseMemory(./memory[1], ctxt,
- def-mem.max_balloon, true)  0)
+ def-mem.max_balloon, true, true)  0)
 goto error;

 if (virDomainParseMemory(./currentMemory[1], ctxt,
- def-mem.cur_balloon, false)  0)
+ def-mem.cur_balloon, false, true)  0)
 goto error;

 /* and info about it */
@@ -12339,19 +12342,19 @@ virDomainDefParseXML(xmlDocPtr xml,

 /* Extract other memory tunables */
 if (virDomainParseMemory(./memtune/hard_limit[1], ctxt,
- def-mem.hard_limit, false)  0)
+ def-mem.hard_limit, false, false)  0)
 goto error;

 if (virDomainParseMemory(./memtune/soft_limit[1], ctxt,
- def-mem.soft_limit, false)  0)
+ def-mem.soft_limit, false, false)  0)
 goto error;

 if (virDomainParseMemory(./memtune/min_guarantee[1], ctxt,
- def-mem.min_guarantee, false)  0)
+ def-mem.min_guarantee, false, false)  0)
 goto error;

 if (virDomainParseMemory(./memtune/swap_hard_limit[1], ctxt,
- def-mem.swap_hard_limit, false)  0)
+ def-mem.swap_hard_limit, false, false)  0)
 goto error;

 n = virXPathULong(string(./vcpu[1]), ctxt, count);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9908d88..fbb3b2f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1966,8 +1966,10 @@ typedef struct _virDomainMemtune virDomainMemtune;
 typedef virDomainMemtune *virDomainMemtunePtr;

 struct _virDomainMemtune {
-unsigned long long max_balloon; /* in kibibytes */
-unsigned long long cur_balloon; /* in kibibytes */
+unsigned

[libvirt] virStorageFileGetMetadata bug?

2014-10-30 Thread Serge Hallyn

Hi,

I'm looking into why virt-aa-helper isn't adding allow rules for
backing stores nested deeper than 1.  So if I do

qemu-img create -f qcow2 l1.img 10G
qemu-img create -f qcow2 -b l1.img l2.img

and use l2.img in a domain, then virt-aa-helper will add allow
rules for the domain to access both l1.img and l2.img.  But if I

qemu-img create -f qcow2 -b l2.img l3.img

and use l3.img in the domain, then l3.img will not get an allow rule.

Looking at src/security/virt-aa-helper.c:get_files(), it is doing:

if (!disk-src-backingStore) {
bool probe = ctl-allowDiskFormatProbing;
virStorageFileGetMetadata(disk-src, -1, -1, probe, false);
}

if (virDomainDiskDefForeachPath(disk, true, add_file_path, buf)  0)
goto cleanup;

and virStorageFileGetMetadata in turn calls virStorageFileGetMetadataRecurse().
So it seems like l3.img *should* be geting hit in virDomainDiskDefForeachPath,
but it's not.  Am I misunderstanding something in how these helpers should be
used?

thanks,
-serge

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

Re: [libvirt] virStorageFileGetMetadata bug?

2014-10-30 Thread Eric Blake

On 10/30/2014 02:32 PM, Serge Hallyn wrote:
 Hi,
 
 I'm looking into why virt-aa-helper isn't adding allow rules for
 backing stores nested deeper than 1.  So if I do
 
 qemu-img create -f qcow2 l1.img 10G
 qemu-img create -f qcow2 -b l1.img l2.img

Oops, you forgot the backing format.  Without that, libvirt is forced to
treat the backing file as raw unless you tweak qemu.conf to allow format
probing (which then exposes you to a CVE if probing ever goes wrong).

Please add -o backing_fmt={qcow2,raw} as appropriate to each qemu-img
create, then try again.

 
 and virStorageFileGetMetadata in turn calls 
 virStorageFileGetMetadataRecurse().
 So it seems like l3.img *should* be geting hit in virDomainDiskDefForeachPath,
 but it's not.  Am I misunderstanding something in how these helpers should be
 used?

You are missing the fact that we refuse to probe a backing file for
format, and instead treat it as raw (even if that treatment is wrong),
unless explicitly configured to be less safe.

-- 
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] virStorageFileGetMetadata bug?

2014-10-30 Thread Serge Hallyn

Quoting Eric Blake (ebl...@redhat.com):
 On 10/30/2014 02:32 PM, Serge Hallyn wrote:
  Hi,
  
  I'm looking into why virt-aa-helper isn't adding allow rules for
  backing stores nested deeper than 1.  So if I do
  
  qemu-img create -f qcow2 l1.img 10G
  qemu-img create -f qcow2 -b l1.img l2.img
 
 Oops, you forgot the backing format.  Without that, libvirt is forced to
 treat the backing file as raw unless you tweak qemu.conf to allow format
 probing (which then exposes you to a CVE if probing ever goes wrong).
 
 Please add -o backing_fmt={qcow2,raw} as appropriate to each qemu-img
 create, then try again.

Jinkeys, yup, that fixes it - thanks!

  and virStorageFileGetMetadata in turn calls 
  virStorageFileGetMetadataRecurse().
  So it seems like l3.img *should* be geting hit in 
  virDomainDiskDefForeachPath,
  but it's not.  Am I misunderstanding something in how these helpers should 
  be
  used?
 
 You are missing the fact that we refuse to probe a backing file for
 format, and instead treat it as raw (even if that treatment is wrong),
 unless explicitly configured to be less safe.

Sounds like the safe thing to do.

thanks,
-serge

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

Re: [libvirt] [PATCH v3 1/3] numatune: add check for numatune nodeset range

2014-10-30 Thread Chen, Fan

On Thu, 2014-10-30 at 07:55 +0100, Martin Kletzander wrote: 
 On Thu, Oct 30, 2014 at 02:23:00AM +, Chen, Fan wrote:
 On Wed, 2014-10-29 at 14:20 +0100, Martin Kletzander wrote:
  On Wed, Oct 29, 2014 at 08:33:32PM +0800, Chen Fan wrote:
  diff --git a/src/util/virnuma.c b/src/util/virnuma.c
  @@ -373,6 +400,12 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED,
  _(NUMA isn't available on this host));
   return -1;
   }
  +
  +bool
  +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
  +{
  +return true;
  +}
   #endif
  
 
  In what case would you like this to return true?
 
 when libvirt does not support numa, we can not check it.
 maybe we should return false if setting nodeset.
 
 
 That was my idea, I just wanted to make sure we're on the same page.
 The thing is that if you want something that's not available, it makes
 more sense to say NO then just allow it because libvirt doesn't
 know.  Make the user fix it :)
Yes, I had made a v4 and sent it, please help to review.

Thanks,
Chen

 
 Martin


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

[libvirt] [python v3 PATCH] Add dict check for setTime and allow pass 'seconds' parameter

2014-10-30 Thread Luyao Huang

When pass None or a empty dictionary to time, it will 
report error.Allow a one-element dictionary which 
contains 'seconds',setting JUST seconds will do the 
sane thing of passing 0 for nseconds, instead of 
erroring out.If dict have a unkown key, it will report error.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
 libvirt-override-virDomain.py |  6 +++---
 libvirt-override.c| 41 +
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py
index a50ec0d..2a4c4c9 100644
--- a/libvirt-override-virDomain.py
+++ b/libvirt-override-virDomain.py
@@ -66,9 +66,9 @@
 if ret == -1: raise libvirtError ('virDomainGetTime() failed', 
dom=self)
 return ret
 
-def setTime(self, time=None, flags=0):
-Set guest time to the given value. @time is a dict conatining
-'seconds' field for seconds and 'nseconds' field for nanosecons 
+def setTime(self, time, flags=0):
+Set guest time to the given value. @time is a dict containing
+'seconds' field for seconds and 'nseconds' field for nanoseconds 
 ret = libvirtmod.virDomainSetTime(self._o, time, flags)
 if ret == -1: raise libvirtError ('virDomainSetTime() failed', 
dom=self)
 return ret
diff --git a/libvirt-override.c b/libvirt-override.c
index a53b46f..1d1714a 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7784,6 +7784,8 @@ static PyObject *
 libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
 PyObject *py_retval = NULL;
 PyObject *pyobj_domain;
+PyObject *pyobj_seconds;
+PyObject *pyobj_nseconds;
 PyObject *py_dict;
 virDomainPtr domain;
 long long seconds = 0;
@@ -7797,25 +7799,40 @@ libvirt_virDomainSetTime(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args) {
 return NULL;
 domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 
-py_dict_size = PyDict_Size(py_dict);
+if (!PyDict_Check(py_dict)) {
+PyErr_Format(PyExc_TypeError, time must be dict);
+return NULL;
+}
 
-if (py_dict_size == 2) {
-PyObject *pyobj_seconds, *pyobj_nseconds;
+py_dict_size = PyDict_Size(py_dict);
 
-if (!(pyobj_seconds = PyDict_GetItemString(py_dict, seconds)) ||
-(libvirt_longlongUnwrap(pyobj_seconds, seconds)  0)) {
-PyErr_Format(PyExc_LookupError, malformed or missing 'seconds');
+if ((pyobj_seconds = PyDict_GetItemString(py_dict, seconds)) != NULL) {
+if (libvirt_longlongUnwrap(pyobj_seconds, seconds)  0) {
+PyErr_Format(PyExc_LookupError, malformed 'seconds');
 goto cleanup;
 }
+} else {
+PyErr_Format(PyExc_LookupError, Dictionary must contains 
+ 'seconds');
+goto cleanup;
+}
 
-if (!(pyobj_nseconds = PyDict_GetItemString(py_dict, nseconds)) ||
-(libvirt_uintUnwrap(pyobj_nseconds, nseconds)  0)) {
-PyErr_Format(PyExc_LookupError, malformed or missing 'nseconds');
+if ((pyobj_nseconds = PyDict_GetItemString(py_dict, nseconds)) != NULL) {
+if (libvirt_uintUnwrap(pyobj_nseconds, nseconds)  0) {
+PyErr_Format(PyExc_LookupError, malformed 'nseconds');
 goto cleanup;
 }
-} else if (py_dict_size  0) {
-PyErr_Format(PyExc_LookupError, Dictionary must contain 
- 'seconds' and 'nseconds');
+} else if (py_dict_size == 1) {
+nseconds = 0;
+} else {
+PyErr_Format(PyExc_LookupError, Dictionary contains 
+ unknown key);
+goto cleanup;
+}
+
+if (py_dict_size  2) {
+PyErr_Format(PyExc_LookupError, Dictionary contains 
+ unknown key);
 goto cleanup;
 }
 
-- 
1.8.3.1

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

[libvirt] virsh migrate

2014-10-30 Thread Chateigner Nicolas


Hi,

I would like to use this command to backup my virtual machine on my 
secondary server


virsh migrate --live --suspend --copy-storage-inc virtual_machine 
qemu+ssh://root@secondary_server/system



But we don't have option to disable shut off the virtual machine on the 
source server.

Would you add an option to offer this functionality ?

Best regards
Nicolas Chateigner



http://www.inserm.fr

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

61 matches

Mail list logo