date:20160412


On Tue, Apr 12, 2016 at 06:24:16PM -0400, TomK wrote:

On 4/12/2016 5:08 PM, John Ferlan wrote:

Having/using a root squash via an NFS pool is "easy" (famous last words)

Create some pool XML (taking the example I have)

% cat nfs.xml

 rootsquash
 
 
 
 
 
 
 /tmp/netfs-rootsquash-pool
 
 0755
 107
 107
 
 


In this case 107:107 is qemu:qemu and I used 'localhost' as the
hostname, but that can be a fqdn or ip-addr to the NFS server.

You've already seen my /etc/exports

virsh pool-define nfs.xml
virsh pool-build rootsquash
virsh pool-start rootsquash
virsh vol-list rootsquash

Now instead of


  
  
  


Something like:

   
 
 
 
   

The volume name may be off, but it's perhaps close.  I forget how to do
the readonly bit for a pool (again, my focus is elsewhere).

Of course you'd have to adjust the nfs.xml above to suit your
environment and see what you see/get.  The privileges for the pool and
volumes in the pool become the key to how libvirt decides to "request
access" to the volume.  "disk.1" having read access is probably not an
issue since you seem to be using it as a CDROM; however, "disk.0" is
going to be used for read/write - thus would have to be appropriately
configured...



Thanks John!  Appreciated again.

No worries, handle what's on the plate now and earmark this for checking
once you have some free cycles.  I can temporarily hop on one leg by
using Martin Kletzander's workaround (It's a POC at the moment).

I'll have a look at your instructions further but wanted to find out if
that config nfs.xml is a one time thing correct?  I'm spinning these up
at will via the OpenNebula GUI and if I have update for each VM, that
breaks the Cloud provisioning.  I'll go over your notes again.  I'm
optimistic.   :)



The more I'm thinking about it, the more I am convinced that the
workaround is actually not a workaround.  The only thing you need to do
is having execute for others (precisely for 'nobody' on the nfs share)
in the whole path on all directories.  Without that even the pool won't
be usable from libvirt.  However it does not pose any security issue as
it only allows others to check the path.  When qemu is launched, it has
the proper "label", meaning uid:gid to access the file so it will be
able to read/write or whatever permissions you set there.  It's just
that libvirt does some checks that the path exists for example.

Hope that's understandable and it will resolve your issue permanently.

Have a nice day,
Martin


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

[libvirt] 答复: How to get GCOV result of make check?

2016-04-12 Thread Zhangbo (Oscar)

>>Hi all:
>>I want to get the coverage result of 'make check', here is my way:
>>1.  ./configure *
>>2.  make clean -j
>>*3*.  find ./ -name Makefile |xargs sed -i "s/\= gcc/= gcc -fprofile-arcs
>-ftest-coverage/g"  # add gcov-related cflags
>>4.  make check -j
>>5.  lcov -d . -c -o libvirt.info
>>6.  genhtml libvirt.info -o libvirtresult
>>
>>My key work Is step 3, however, I don't think it's a graceful way to get
>gcov result.
>>How do you usually get the gcov results, thanks!
>>
>
>check './configure --help', look for '--enable-test-coverage'
>

Great, It works, and we found that 'make coverage' works as well , Thank you 
very much.
>>Oscar.
>>
>>--

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

Re: [libvirt] [PATCH] tests: Fix VIR_TEST_REGENERATE_OUTPUT

2016-04-12 Thread Laine Stump


On 04/12/2016 06:33 PM, Cole Robinson wrote:

commit 950a90d489 mocked some virCommand handling for the qemu tests,
but we were using that in the test suite to call test-wrap-argv.pl for
regenerating test output.

Switch the generator code to just use system() instead
---


I dislike giving up the potential utility of virCommand*() in tests, but 
this does fix the problem (without creating the inefficiency of an extra 
function in the callstack for every virCommandRun() call by every 
libvirtd in the world). So ACK unless someone has a better idea / 
different opinion. (But there's one thing to fix, noted below).


(if this was in a binary run outside the build environment, I would NACK 
use of system(), but since it's only used by a test...)




  tests/testutils.c | 19 +--
  1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index a0ce4b6..4f3e67b 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -434,24 +434,15 @@ virTestRewrapFile(const char *filename)
  {
  int ret = -1;
  char *outbuf = NULL;


outbuf is no longer used, so it should be removed.


-char *script = NULL;
-virCommandPtr cmd = NULL;
+char *cmd = NULL;
  
-if (virAsprintf(, "%s/test-wrap-argv.pl", abs_srcdir) < 0)

+if (virAsprintf(, "echo \"$(%s/test-wrap-argv.pl %s)\" > %s",
+abs_srcdir, filename, filename) < 0)
  goto cleanup;


For that matter, cmd would be NULL if virAsprintf failed, so all that's 
really needed here is a "return -1;", meaning that the cleanup label can 
also be removed.


  
-cmd = virCommandNewArgList(script, filename, NULL);

-virCommandSetOutputBuffer(cmd, );
-if (virCommandRun(cmd, NULL) < 0)
-goto cleanup;
-
-if (virFileWriteStr(filename, outbuf, 0666) < 0)
-goto cleanup;
-
-ret = 0;
+ret = system(cmd);
   cleanup:
-VIR_FREE(script);
-virCommandFree(cmd);
+VIR_FREE(cmd);
  VIR_FREE(outbuf);
  return ret;
  }


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

Re: [libvirt] [PATCH 7/7] qemu: process: Wire up ACPI OST events to notify users of failed memory unplug

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Since qemu is now able to notify us that the guest rejected the memory
> unplug operation we can relay this to the user and make the API fail
> right away.
> 
> Additionally document the possible values from the ACPI docs for future
> reference.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1320447

ACK, nice series. So do the ACPI bits give us info about any other device
hotplug failures that can be added later? Or is this only for memory hotplug?

- Cole

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

Re: [libvirt] [PATCH 6/7] qemu: monitor: Add support for ACPI_DEVICE_OST event handling

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> The event is emitted on ACPI OSPM Status Indication events.
> 
> ACPI standard documentation describes the method as:
> 
> This object is an optional control method that is invoked by OSPM to
> indicate processing status to the platform. During device ejection,
> device hot add, or other event processing, OSPM may need to perform
> specific handshaking with the platform. OSPM may also need to indicate
> to the platform its inability to complete a requested operation; for
> example, when a user presses an ejection button for a device that is
> currently in use or is otherwise currently incapable of being ejected.
> In this case, the processing of the ACPI Eject Request notification by
> OSPM fails. OSPM may indicate this failure to the platform through the
> invocation of the _OST control method. As a result of the status
> notification indicating ejection failure, the platform may take certain
> action including reissuing the notification or perhaps turning on an
> appropriate indicator light to signal the failure to the user.

ACK

- Cole

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

Re: [libvirt] [PATCH 5/7] Add VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Since we didn't opt to use one single event for device lifecycle for a
> VM we are missing one last event if the device removal failed. This
> event will be emitted once we asked to eject the device but for some
> reason it is not possible.
> ---
>  daemon/remote.c  | 36 +
>  include/libvirt/libvirt-domain.h | 21 ++
>  src/conf/domain_event.c  | 84 
> 
>  src/conf/domain_event.h  |  7 
>  src/libvirt_private.syms |  2 +
>  src/remote/remote_driver.c   | 30 ++
>  src/remote/remote_protocol.x | 14 ++-
>  src/remote_protocol-structs  |  6 +++
>  tools/virsh-domain.c | 18 +
>  9 files changed, 217 insertions(+), 1 deletion(-)

ACK, I verified it largely mirrors DeviceAdded signal.

event-test.c follow up patch?

- Cole

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

Re: [libvirt] [PATCH 4/7] qemu: hotplug: Add support for signalling device unplug failure

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Similarly to the DEVICE_DELETED event we will be able to tell when
> unplug of certain device types will be rejected by the guest OS. Wire up
> the device deletion signalling code to allow handling this.
> ---
>  src/qemu/qemu_domain.h  | 17 -
>  src/qemu/qemu_hotplug.c | 30 +-
>  src/qemu/qemu_hotplug.h |  3 ++-
>  src/qemu/qemu_process.c |  2 +-
>  4 files changed, 40 insertions(+), 12 deletions(-)
> 

There's a slight conflict here with git master but it should rebase cleanly,
git am --3way couldn't handle it for me for whatever reason

> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 2b92a90..84b4b16 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -148,6 +148,20 @@ struct qemuDomainJobObj {
>  typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver,
>virDomainObjPtr vm);
> 
> +/* helper data types for async device unplug */
> +typedef enum {
> +QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_NONE = 0,
> +QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_OK,
> +QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_GUEST_REJECTED,
> +} qemuDomainUnplugDeviceStatus;
> +
> +typedef struct _qemuDomainUnplugDevice qemuDomainUnplugDevice;
> +typedef qemuDomainUnplugDevice *qemuDomainUnplugDevicePtr;
> +struct _qemuDomainUnplugDevice {
> +const char *alias;
> +qemuDomainUnplugDeviceStatus status;
> +};
> +
>  typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
>  typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
>  struct _qemuDomainObjPrivate {
> @@ -198,7 +212,8 @@ struct _qemuDomainObjPrivate {
> 
>  virPerfPtr perf;
> 
> -const char *unpluggingDevice; /* alias of the device that is being 
> unplugged */
> +qemuDomainUnplugDevice unplug;
> +

FWIW in isolation I'd think 'qemuDomainUnplugDevice' was a function name...
maybe stick with the UnpluggingDevice name. Not a NACK, just an idea if you're
motivated. Also lack Ptr use just looks weird, I'm so used to everything being
a Ptr in libvirt code :)

>  char **qemuDevices; /* NULL-terminated list of devices aliases known to 
> QEMU */
> 
>  bool hookRun;  /* true if there was a hook run over this domain */
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 134f458..bd0599f 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3337,20 +3337,24 @@ qemuDomainMarkDeviceForRemoval(virDomainObjPtr vm,
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> 
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
> -priv->unpluggingDevice = info->alias;
> -else
> -priv->unpluggingDevice = NULL;
> +memset(>unplug, 0, sizeof(priv->unplug));
> +
> +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
> +return;
> +
> +priv->unplug.alias = info->alias;
>  }
> 
>  static void
>  qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> -priv->unpluggingDevice = NULL;
> +priv->unplug.alias = NULL;
>  }
> 
>  /* Returns:
> + *  -1 Unplug of the device failed
> + *
>   *   0 DEVICE_DELETED event is supported and removal of the device did not
>   * finish in qemuDomainRemoveDeviceWaitTime
>   *
> @@ -3373,17 +3377,23 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>  return 1;
>  until += qemuDomainRemoveDeviceWaitTime;
> 
> -while (priv->unpluggingDevice) {
> +while (priv->unplug.alias) {
>  if ((rc = virDomainObjWaitUntil(vm, until)) == 1)
>  return 0;
> 
>  if (rc < 0) {
>  VIR_WARN("Failed to wait on unplug condition for domain '%s' "
> - "device '%s'", vm->def->name, priv->unpluggingDevice);
> + "device '%s'", vm->def->name, priv->unplug.alias);
>  return 1;
>  }
>  }
> 
> +if (priv->unplug.status == 
> QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_GUEST_REJECTED) {
> +virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +   _("unplug of device was rejected by the guest"));
> +return -1;
> +}
> +
>  return 1;
>  }
> 

Okay, so 0 == didn't succeed in time, 1 == definitely succeeded OR we have no
way of knowing so assume it worked, -1 == definitely failed. That's a lot of
semantics :) But this makes sense, now when the -1 trickles up the stack, it's
when we know for certain that the hotplug failed.

ACK

- Cole

> @@ -3395,12 +3405,14 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>   */
>  bool
>  qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
> -  const char *devAlias)
> +  const char *devAlias,
> +  qemuDomainUnplugDeviceStatus status)
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> 
> -if (STREQ_NULLABLE(priv->unpluggingDevice,

Re: [libvirt] [PATCH 3/7] qemu: Use domain condition for device removal singalling

s/singalling/signaling/

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> No need to keep two separate conditions. A slight juggling of return
> values is needed to accomodate virDomainObjWaitUntil.
> ---
>  src/qemu/qemu_domain.c  |  4 
>  src/qemu/qemu_domain.h  |  1 -
>  src/qemu/qemu_hotplug.c | 19 +--
>  3 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f38b0f3..8a673f8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -586,9 +586,6 @@ qemuDomainObjPrivateAlloc(void)
>  goto error;
>  }
> 
> -if (virCondInit(>unplugFinished) < 0)
> -goto error;
> -
>  if (!(priv->devs = virChrdevAlloc()))
>  goto error;
> 
> @@ -618,7 +615,6 @@ qemuDomainObjPrivateFree(void *data)
>  VIR_FREE(priv->lockState);
>  VIR_FREE(priv->origname);
> 
> -virCondDestroy(>unplugFinished);
>  virStringFreeList(priv->qemuDevices);
>  virChrdevFree(priv->devs);
> 
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 54d7bd7..2b92a90 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -198,7 +198,6 @@ struct _qemuDomainObjPrivate {
> 
>  virPerfPtr perf;
> 
> -virCond unplugFinished; /* signals that unpluggingDevice was unplugged */
>  const char *unpluggingDevice; /* alias of the device that is being 
> unplugged */
>  char **qemuDevices; /* NULL-terminated list of devices aliases known to 
> QEMU */
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7317089..134f458 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3364,6 +3364,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
>  unsigned long long until;
> +int rc;
> 
>  if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
>  return 1;
> @@ -3373,15 +3374,13 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>  until += qemuDomainRemoveDeviceWaitTime;
> 
>  while (priv->unpluggingDevice) {
> -if (virCondWaitUntil(>unplugFinished,
> - >parent.lock, until) < 0) {
> -if (errno == ETIMEDOUT) {
> -return 0;
> -} else {
> -VIR_WARN("Failed to wait on unplug condition for domain '%s' 
> "
> - "device '%s'", vm->def->name, 
> priv->unpluggingDevice);
> -return 1;
> -}
> +if ((rc = virDomainObjWaitUntil(vm, until)) == 1)
> +return 0;
> +
> +if (rc < 0) {
> +VIR_WARN("Failed to wait on unplug condition for domain '%s' "
> + "device '%s'", vm->def->name, priv->unpluggingDevice);
> +return 1;
>  }
>  }
> 
> @@ -3402,7 +3401,7 @@ qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
> 
>  if (STREQ_NULLABLE(priv->unpluggingDevice, devAlias)) {
>  qemuDomainResetDeviceRemoval(vm);
> -virCondSignal(>unplugFinished);
> +virDomainObjBroadcast(vm);
>  return true;
>  }
>  return false;
> 

Neat, I didn't know about virDomainObjBroadcast etc. LGTM, ACK

- Cole

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

Re: [libvirt] [PATCH 2/7] qemu: hotplug: Refactor semantics of qemuDomainWaitForDeviceRemoval

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Neither of the callers cares whether the DEVICE_DELETED event isn't
> supported or the event was received. Simplify the code and callers by
> unifying the two values and changing the return value constants so that
> a temporary variable can be omitted.
> ---
>  src/qemu/qemu_hotplug.c | 67 
> +++--
>  1 file changed, 20 insertions(+), 47 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6c619e9..7317089 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3351,11 +3351,13 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
>  }
> 
>  /* Returns:
> - *   0 when DEVICE_DELETED event is unsupported, or we failed to reliably 
> wait
> - *   for the event
> - *   1 when DEVICE_DELETED arrived before the timeout and the caller is
> - * responsible for finishing the removal
> - *   2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
> + *   0 DEVICE_DELETED event is supported and removal of the device did not
> + * finish in qemuDomainRemoveDeviceWaitTime
> + *
> + *   1 when the caller is responsible for finishing the device removal:
> + *  - DEVICE_DELETED event is unsupported
> + *  - DEVICE_DELETED event arrived before the timeout time
> + *  - we failed to reliably wait for the event and thus use fallback 
> behavior
>   */

Makes sense... return 1 basically means 'removal succeeded OR there's no way
we can tell if it succeeded or not, so just update the XML to assume it did'

ACK

- Cole

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

Re: [libvirt] [PATCH 1/7] qemu: hotplug: Properly handle errors in qemuDomainWaitForDeviceRemoval

On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Callers ignore if this function returns -1 and continue as though the
> DEVICE_DELETED event was not received. Since we can't be sure that the
> event was not received we should behave as if the event was not
> supported and remove the device definition right away. The error
> fortunately won't really happen here.
> ---
>  src/qemu/qemu_hotplug.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 48bea6a..6c619e9 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3351,8 +3351,8 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
>  }
> 
>  /* Returns:
> - *  -1 on error
> - *   0 when DEVICE_DELETED event is unsupported
> + *   0 when DEVICE_DELETED event is unsupported, or we failed to reliably 
> wait
> + *   for the event
>   *   1 when DEVICE_DELETED arrived before the timeout and the caller is
>   * responsible for finishing the removal
>   *   2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
> @@ -3367,7 +3367,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>  return 0;
> 
>  if (virTimeMillisNow() < 0)
> -return -1;
> +return 0;
>  until += qemuDomainRemoveDeviceWaitTime;
> 
>  while (priv->unpluggingDevice) {
> @@ -3376,9 +3376,9 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
>  if (errno == ETIMEDOUT) {
>  return 2;
>  } else {
> -virReportSystemError(errno, "%s",
> - _("Unable to wait on unplug 
> condition"));
> -return -1;
> +VIR_WARN("Failed to wait on unplug condition for domain '%s' 
> "
> + "device '%s'", vm->def->name, 
> priv->unpluggingDevice);
> +return 0;
>  }
>  }
>  }
> 

Makes sense, and I verified every caller is just checking the return value ==
1. ACK

- Cole

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

Re: [libvirt] [PATCH 12/12] qemu: Utilize qemu secret objects for SCSI/RBD auth/secret

[...]

>  configure.ac|   1 +
>  src/qemu/qemu_alias.c   |  23 ++
>  src/qemu/qemu_alias.h   |   2 +
>  src/qemu/qemu_command.c | 126 ++
>  src/qemu/qemu_command.h |   4 +
>  src/qemu/qemu_domain.c  | 199 
> +---
>  src/qemu/qemu_domain.h  |   3 +
>  src/qemu/qemu_hotplug.c |  72 +-
>  8 files changed, 418 insertions(+), 12 deletions(-)
> 

Digging in deeper to the testing portion proves to me more is needed on
this patch - in particular qemuDomainSecretIVSetup has a couple of
issues. One being, I needed to do an "if (secret)" in the cleanup
section since I VIR_FREE'd it earlier. It was all cleanup, then I
thought - probably should clear/free the secret right after using - bad
idea ;-)...  I also have something not quite right because when the
object is passed to qemu and the decrypt is done, the answer isn't
correct - even though if I "add" a decrypt call and check the result, I
get the right answer.  Perhaps something to do with the padding - still
not quite sure.

Also, I neglected to think about hotunplug and removing the object...

John

[...]

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

[libvirt] [PATCH v3] util: Add virGettextInitialize, convert the code

Take setlocale/gettext error handling pattern from tools/virsh-*
and use it for all standalone binaries via a new shared
virGettextInitialize routine. The virsh* pattern differed slightly
from other callers. All users now consistently:

* Ignore setlocale errors. virsh has done this forever, presumably for
  good reason. This has been partially responsible for some bug reports:

  https://bugzilla.redhat.com/show_bug.cgi?id=1312688
  https://bugzilla.redhat.com/show_bug.cgi?id=1026514
  https://bugzilla.redhat.com/show_bug.cgi?id=1016158

* Report the failed function name
* Report strerror
---
v3:
Fix make syntax-check; add a rule similar to the bindtextdomain
check that checks for the new virGettextInitialize

 cfg.mk| 11 +
 daemon/libvirtd.c |  6 ++---
 src/Makefile.am   |  2 ++
 src/libvirt_private.syms  |  4 
 src/locking/lock_daemon.c |  6 ++---
 src/locking/sanlock_helper.c  |  9 ++-
 src/logging/log_daemon.c  |  6 ++---
 src/lxc/lxc_controller.c  |  6 ++---
 src/network/leaseshelper.c| 12 +++---
 src/security/virt-aa-helper.c | 12 +++---
 src/storage/parthelper.c  |  9 ++-
 src/util/iohelper.c   | 13 +++---
 src/util/virgettext.c | 56 +++
 src/util/virgettext.h | 25 +++
 tools/virsh.c | 15 ++--
 tools/virt-admin.c| 15 ++--
 tools/virt-host-validate.c| 15 ++--
 tools/virt-login-shell.c  | 14 ++-
 tools/vsh.c   |  2 --
 19 files changed, 127 insertions(+), 111 deletions(-)
 create mode 100644 src/util/virgettext.c
 create mode 100644 src/util/virgettext.h

diff --git a/cfg.mk b/cfg.mk
index 5372584..90bb709 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1037,6 +1037,15 @@ sc_prohibit_verbose_strcat:
halt='Use strcat(a, b) instead of strncat(a, b, strlen(b))' \
  $(_sc_search_regexp)
 
+# Ensure that each .c file containing a "main" function also
+# calls virGettextInitialize
+sc_gettext_init:
+   @require='virGettextInitialize *\(' 
\
+   in_vc_files='\.c$$' \
+   containing='\
 #include 
 #include 
-#include 
 
 #include "libvirt_internal.h"
 #include "virerror.h"
@@ -58,6 +57,7 @@
 #include "locking/lock_manager.h"
 #include "viraccessmanager.h"
 #include "virutil.h"
+#include "virgettext.h"
 
 #ifdef WITH_DRIVER_MODULES
 # include "driver.h"
@@ -1172,9 +1172,7 @@ int main(int argc, char **argv) {
 {0, 0, 0, 0}
 };
 
-if (setlocale(LC_ALL, "") == NULL ||
-bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-textdomain(PACKAGE) == NULL ||
+if (virGettextInitialize() < 0 ||
 virInitialize() < 0) {
 fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
 exit(EXIT_FAILURE);
diff --git a/src/Makefile.am b/src/Makefile.am
index eda0365..38b9560 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -114,6 +114,7 @@ UTIL_SOURCES =  
\
util/virfile.c util/virfile.h   \
util/virfirewall.c util/virfirewall.h   \
util/virfirewallpriv.h  \
+   util/virgettext.c util/virgettext.h \
util/virgic.c util/virgic.h \
util/virhash.c util/virhash.h   \
util/virhashcode.c util/virhashcode.h   \
@@ -2321,6 +2322,7 @@ libvirt_setuid_rpc_client_la_SOURCES =\
util/virevent.c \
util/vireventpoll.c \
util/virfile.c  \
+   util/virgettext.c   \
util/virhash.c  \
util/virhashcode.c  \
util/virjson.c  \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 068bc00..af7de8a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1517,6 +1517,10 @@ virFirewallStartRollback;
 virFirewallStartTransaction;
 
 
+# util/virgettext.h
+virGettextInitialize;
+
+
 # util/virgic.h
 virGICVersionTypeFromString;
 virGICVersionTypeToString;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 973e691..bfdcfc6 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -28,7 +28,6 @@
 #include 
 #include 
 #include 
-#include 
 
 
 #include "lock_daemon.h"
@@ -47,6 +46,7 @@
 #include "virhash.h"
 #include "viruuid.h"
 #include "virstring.h"
+#include "virgettext.h"
 
 #include "locking/lock_daemon_dispatch.h"
 #include "locking/lock_protocol.h"
@@ -1179,9 +1179,7 @@ int main(int argc, char **argv) {
 
 privileged = geteuid() == 0;
 
-if (setlocale(LC_ALL, "") == NULL ||

Re: [libvirt] VIR_TEST_REGENERATE_OUTPUT is broken (Was: [PATCH] qemuxml2argvtest: Adapt to ethernet automatic tap creation)

On 04/12/2016 05:48 PM, Laine Stump wrote:
> On 03/23/2016 11:35 AM, Michal Privoznik wrote:
>> After 9c17d665fdc5 the tap device for ethernet network type is
>> automatically precreated before spawning qemu. Problem is, the
>> qemuxml2argvtest wasn't updated and thus is failing. Because of
>> all the APIs that new code is calling, I had to mock a lot. Also,
>> since the tap FDs are labeled separately from the rest of the
>> devices/files I had to enable NOP security driver for the test
>> too.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
> 
>> --- a/tests/qemuxml2argvmock.c
>> +++ b/tests/qemuxml2argvmock.c
> 
>> +}
>> +
>> +int
>> +virCommandRun(virCommandPtr cmd ATTRIBUTE_UNUSED,
>> +  int *exitstatus)
>> +{
>> +if (exitstatus)
>> +*exitstatus = 0;
>> +
>> +return 0;
>> +}
> 
> 
> The problem with mocking virCommandRun is that it is called by the test
> infrastructure (virTestRewrapFile(), which is used when regenerating the test
> results (VIR_TEST_REGENERATE_OUTPUT=1)).
> 
> After this commit that function silently fails, which results in
> virFileWriteStr() calling strlen(NULL) and a crash of the test. (Nobody
> noticed this before because it's only called if you set
> VIR_TEST_REGENERATE_OUTPUT *and* the results of one of the qemuxml2argv tests
> has changed).
> 
> So what's the most reasonable way to deal with this? I suppose we could rename
> virCommandRun to e.g. virCommandRunInternal() which would be called by a new
> virCommandRun(), then have virTestRewrapFile() call virCommandRunInternal() so
> that it wouldn't get the mocked version. That seems ugly, inefficient, and
> hackish, but I can't think of any way that isn't ugly, inefficient, and
> hackish...
> 

I can't really decide if mocking virCommand is good or not. On one hand it'll
catch any virCommand calls in driver code that may leak into the test suite
which mean host environment dependencies, on the other hand losing the ability
to use virCommand in the test suite might be a problem some day.

Either way we can work around this issue easily enough by just using system()
to call test-wrap-argv.pl, I've sent a patch

- Cole

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

[libvirt] [PATCH] tests: Fix VIR_TEST_REGENERATE_OUTPUT

commit 950a90d489 mocked some virCommand handling for the qemu tests,
but we were using that in the test suite to call test-wrap-argv.pl for
regenerating test output.

Switch the generator code to just use system() instead
---
 tests/testutils.c | 19 +--
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index a0ce4b6..4f3e67b 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -434,24 +434,15 @@ virTestRewrapFile(const char *filename)
 {
 int ret = -1;
 char *outbuf = NULL;
-char *script = NULL;
-virCommandPtr cmd = NULL;
+char *cmd = NULL;
 
-if (virAsprintf(, "%s/test-wrap-argv.pl", abs_srcdir) < 0)
+if (virAsprintf(, "echo \"$(%s/test-wrap-argv.pl %s)\" > %s",
+abs_srcdir, filename, filename) < 0)
 goto cleanup;
 
-cmd = virCommandNewArgList(script, filename, NULL);
-virCommandSetOutputBuffer(cmd, );
-if (virCommandRun(cmd, NULL) < 0)
-goto cleanup;
-
-if (virFileWriteStr(filename, outbuf, 0666) < 0)
-goto cleanup;
-
-ret = 0;
+ret = system(cmd);
  cleanup:
-VIR_FREE(script);
-virCommandFree(cmd);
+VIR_FREE(cmd);
 VIR_FREE(outbuf);
 return ret;
 }
-- 
2.7.3

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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path


On 4/12/2016 5:08 PM, John Ferlan wrote:


On 04/12/2016 03:55 PM, TomK wrote:

On 4/12/2016 3:40 PM, Martin Kletzander wrote:

[ I would be way easier to reply if you didn't top-post ]

On Tue, Apr 12, 2016 at 12:07:50PM -0400, TomK wrote:

Hey John,

Hehe, I got the right guy then.  Very nice!  And very good ideas but I
may need more time to reread and try them out later tonight. I'm fully
in agreement about providing more details.  Can't be accurate in a
diagnosis if there isn't much data to go on.  This pool option is new to
me.  Please tell me more on it.  Can't find it in the file below but
maybe it's elsewhere?

(  ) perhaps rather than the "NFS" pool ( e.g.  )


Allright, here's the details:

[root@mdskvm-p01 ~]# rpm -aq|grep -i libvir
libvirt-daemon-driver-secret-1.2.17-13.el7_2.4.x86_64
libvirt-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-network-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-lxc-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-interface-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-network-1.2.17-13.el7_2.4.x86_64
libvirt-client-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-qemu-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-storage-1.2.17-13.el7_2.4.x86_64
libvirt-python-1.2.17-2.el7.x86_64
libvirt-glib-0.1.9-1.el7.x86_64
libvirt-daemon-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nodedev-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-kvm-1.2.17-13.el7_2.4.x86_64
[root@mdskvm-p01 ~]# cat /etc/release
cat: /etc/release: No such file or directory
[root@mdskvm-p01 ~]# cat /etc/*release*
NAME="Scientific Linux"
VERSION="7.2 (Nitrogen)"
ID="rhel"
ID_LIKE="fedora"
VERSION_ID="7.2"
PRETTY_NAME="Scientific Linux 7.2 (Nitrogen)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:scientificlinux:scientificlinux:7.2:GA"
HOME_URL="http://www.scientificlinux.org//;
BUG_REPORT_URL="mailto:scientific-linux-de...@listserv.fnal.gov;

REDHAT_BUGZILLA_PRODUCT="Scientific Linux 7"
REDHAT_BUGZILLA_PRODUCT_VERSION=7.2
REDHAT_SUPPORT_PRODUCT="Scientific Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="7.2"
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
cpe:/o:scientificlinux:scientificlinux:7.2:ga
[root@mdskvm-p01 ~]#

[root@mdskvm-p01 ~]# mount /var/lib/one
[root@mdskvm-p01 ~]# su - oneadmin
Last login: Sat Apr  9 10:39:25 EDT 2016 on pts/0
Last failed login: Tue Apr 12 12:00:57 EDT 2016 from opennebula01 on
ssh:notty
There were 9584 failed login attempts since the last successful login.
i[oneadmin@mdskvm-p01 ~]$ id oneadmin
uid=9869(oneadmin) gid=9869(oneadmin)
groups=9869(oneadmin),992(libvirt),36(kvm)
[oneadmin@mdskvm-p01 ~]$ pwd
/var/lib/one
[oneadmin@mdskvm-p01 ~]$ ls -altriR|grep -i root
134320262 drwxr-xr-x. 45 root root4096 Apr 12 07:58 ..
[oneadmin@mdskvm-p01 ~]$



It'd take more time than I have at the present moment to root out what
changed when for NFS root-squash, but suffice to say there were some
corner cases.  Some involving how qemu-img files are generated - I don't
have the details present in my short term memory.


[oneadmin@mdskvm-p01 ~]$ cat /var/lib/one//datastores/0/38/deployment.0

 one-38
 1
 
 1024
 
 524288
 
 hvm
 
 
 
/usr/libexec/qemu-kvm
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 


[oneadmin@mdskvm-p01 ~]$ cat
/var/lib/one//datastores/0/38/deployment.0|grep -i nfs
[oneadmin@mdskvm-p01 ~]$


Having/using a root squash via an NFS pool is "easy" (famous last words)

Create some pool XML (taking the example I have)

% cat nfs.xml

 rootsquash
 
 
 
 
 
 
 /tmp/netfs-rootsquash-pool
 
 0755
 107
 107
 
 


In this case 107:107 is qemu:qemu and I used 'localhost' as the
hostname, but that can be a fqdn or ip-addr to the NFS server.

You've already seen my /etc/exports

virsh pool-define nfs.xml
virsh pool-build rootsquash
virsh pool-start rootsquash
virsh vol-list rootsquash

Now instead of


  
  
  


Something like:

   
 
 
 
   

The volume name may be off, but it's perhaps close.  I forget how to do
the readonly bit for a pool (again, my focus is elsewhere).

Of course you'd have to adjust the nfs.xml above to suit your
environment and see what you see/get.  The privileges for the pool and
volumes in the pool become the key to how libvirt decides to "request
access" to the volume.  "disk.1" having

Re: [libvirt] [PATCH 0/2] host-validate: Don't build on Windows

On 04/12/2016 01:07 PM, Andrea Bolognani wrote:
> On Fri, 2016-04-08 at 14:03 -0400, Cole Robinson wrote:
>> On 04/08/2016 12:04 PM, Andrea Bolognani wrote:
>>>
>>> The recent fixes to virt-host-validate have caused the mingw
>>> build to fail[1].
>>>
>>> Instead of working around Windows' quirks in the tool, just
>>> stop building it altogether - it's not like it made any
>>> sense to have it available on that OS anyway.
>>>
>>> Cheers.
>>>
>>>
>>> [1] https://ci.centos.org/view/libvirt-project/job/libvirt-mingw/1496/
>>>
>>> Andrea Bolognani (2):
>>>   tools: Reorganize conditional bits
>>>   configure: Make virt-host-validate optional
>>>
>>>  configure.ac | 29 ++---
>>>  m4/virt-host-validate.m4 | 40 
>>>  tools/Makefile.am| 35 ++-
>>>  3 files changed, 76 insertions(+), 28 deletions(-)
>>>  create mode 100644 m4/virt-host-validate.m4
>>
>> I applied them both, tried 'make distcheck', and hit:
>>
>>  /usr/bin/install -c -m 644 ../../../tools/virt-host-validate.1
>> ../../../tools/virt-pki-validate.1 ../../../tools/virt-xml-validate.1
>> ../../../tools/virsh.1 ../../../tools/virt-admin.1
>> ../../../tools/virt-login-shell.1 ../../../tools/virt-host-validate.1
>> '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1'
>> /usr/bin/install: will not overwrite just-created
>> '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1/virt-host-validate.1'
>> with '../../../tools/virt-host-validate.1'
>> Makefile:2863: recipe for target 'install-man1' failed
>> make[4]: *** [install-man1] Error 1
>> make[4]: *** Waiting for unfinished jobs
>>
>> Makefiles suck :)
> 
> I just realized Martin didn't include you when replying.
> 
> Would you mind trying again after squashing in the hunk
> from[1]? That should fix the failure you're seeing.

Yup, works for my test case. ACK

Thanks,
Cole

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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path


On 4/12/2016 4:36 PM, Martin Kletzander wrote:

On Tue, Apr 12, 2016 at 10:29:29PM +0200, Martin Kletzander wrote:

On Tue, Apr 12, 2016 at 03:55:45PM -0400, TomK wrote:

On 4/12/2016 3:40 PM, Martin Kletzander wrote:

[ I would be way easier to reply if you didn't top-post ]

On Tue, Apr 12, 2016 at 12:07:50PM -0400, TomK wrote:

On 4/12/2016 11:45 AM, John Ferlan wrote:

What got my attention was the error message "initializing FS storage
file" with the "file:" prefix to the name and 9869:9869 as the 
uid:gid

trying to access the file (I assume that's oneadmin:oneadmin on your
system).



I totally missed this.  So the only thing that popped on my mind 
now was

checking the whole path:

 ls -ld /var{,/lib{,/one{,/datastores{,/0{,/38{,/disk.1}}

You can also run it as root and oneadmin, however after reading 
through

all the info again, I don't think that'll help.

I top post by default in thunderbird and we have same setup at work 
with

M$ LookOut.  Old habits are to blame I guess.  I'll try to reply  like
this instead.  But yeah it's terrible for mailing lists to top post.
Here's the output and thanks again:

[oneadmin@mdskvm-p01 ~]$ ls -ld
/var{,/lib{,/one{,/datastores{,/0{,/38{,/disk.1}}
drwxr-xr-x. 21 root root   4096 Apr 11 07:10 /var
drwxr-xr-x. 45 root root   4096 Apr 12 07:58 /var/lib
drwxr-x---  12 oneadmin oneadmin   4096 Apr 12 15:50 /var/lib/one


Look ^^, maybe for a quick workaround you could try doing:

 chmod o+rx /var/lib/one



Actually, o+x ought to be enough.


Let me know if that does the trick (at least for now).

drwxrwxr-x   6 oneadmin oneadmin 46 Mar 31 02:44 
/var/lib/one/datastores

drwxrwxr-x   6 oneadmin oneadmin 42 Apr  5 00:20
/var/lib/one/datastores/0
drwxrwxr-x   2 oneadmin oneadmin 68 Apr  5 00:20
/var/lib/one/datastores/0/38
-rw-r--r--   1 oneadmin oneadmin 372736 Apr  5 00:20
/var/lib/one/datastores/0/38/disk.1
[oneadmin@mdskvm-p01 ~]$

That's the default setting but I think I see what you're getting at 
that

permissions get inherited?



No, I just think you need eXecute on all parent directories. That
shouldn't hinder your security and could help.


Cheers,
Tom K.
- 




Living on earth is expensive, but it includes a free trip around the 
sun.







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



___
libvirt-users mailing list
libvirt-us...@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users


The execute permissions did the trick to allow creation.  So that's 
good.  There's still the write and I'm thinking you intend this as a 
workaround since oneadmin should be able to write in there with other 
being --- .  The auto deployment of cloud virtuals would still fail then 
when writes are attempted.


[oneadmin@mdskvm-p01 ~]$ virsh -d 1 --connect qemu:///system create 
/var/lib/one//datastores/0/38/deployment.0

create: file(optdata): /var/lib/one//datastores/0/38/deployment.0
Domain one-38 created from /var/lib/one//datastores/0/38/deployment.0
[oneadmin@mdskvm-p01 ~]$

Now should this work without any permissions on other for the 
unprivileged user oneadmin?  Thinking Yes per John Forlan's reply?


[oneadmin@mdskvm-p01 0]$ virsh -d 1 --connect qemu:///system create 
/var/lib/one//datastores/0/24/deployment.0

create: file(optdata): /var/lib/one//datastores/0/24/deployment.0
error: Failed to create domain from 
/var/lib/one//datastores/0/24/deployment.0
error: can't canonicalize path '/var/lib/one//datastores/0/24/disk.1': 
Permission denied

[oneadmin@mdskvm-p01 0]$


Cheers,
Tom K.
- 


Living on earth is expensive, but it includes a free trip around the sun.

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

Re: [libvirt] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

2016-04-12 Thread Wei Liu

On Tue, Apr 12, 2016 at 03:31:46PM -0600, Jim Fehlig wrote:
> Wei Liu wrote:
> > Hi libvirt maintainers,
> 
> Sorry for the delay. Slowly catching up on mail after vacation...
> 
> > 
> > Xen's control library libxenlight (libxl) requires application
> > (libvirt in this case) to explictily define LIBXL_API_VERSION.
> 
> Where is this requirement written? :-)
> 
> > This is
> > lacking at the moment so libvirt's libxl driver always gets the latest
> > APIs.
> 
> IMO, that is what we want for upstream libvirt. Downstreams can choose a
> specific version if they want.
> 
> > We changed one of the APIs in libxl so libvirt's libxl driver
> > can't build at the moment.
> 
> A quick glance ahead in my libvirt mail tells me you fixed this with the
> preferred LIBXL_HAVE_* pattern.
> 

It's already done that way. :-)

Wei.

> Regards,
> Jim

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

[libvirt] VIR_TEST_REGENERATE_OUTPUT is broken (Was: [PATCH] qemuxml2argvtest: Adapt to ethernet automatic tap creation)

2016-04-12 Thread Laine Stump


On 03/23/2016 11:35 AM, Michal Privoznik wrote:

After 9c17d665fdc5 the tap device for ethernet network type is
automatically precreated before spawning qemu. Problem is, the
qemuxml2argvtest wasn't updated and thus is failing. Because of
all the APIs that new code is calling, I had to mock a lot. Also,
since the tap FDs are labeled separately from the rest of the
devices/files I had to enable NOP security driver for the test
too.

Signed-off-by: Michal Privoznik 
---



--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c



+}
+
+int
+virCommandRun(virCommandPtr cmd ATTRIBUTE_UNUSED,
+  int *exitstatus)
+{
+if (exitstatus)
+*exitstatus = 0;
+
+return 0;
+}



The problem with mocking virCommandRun is that it is called by the test 
infrastructure (virTestRewrapFile(), which is used when regenerating the 
test results (VIR_TEST_REGENERATE_OUTPUT=1)).


After this commit that function silently fails, which results in 
virFileWriteStr() calling strlen(NULL) and a crash of the test. (Nobody 
noticed this before because it's only called if you set 
VIR_TEST_REGENERATE_OUTPUT *and* the results of one of the qemuxml2argv 
tests has changed).


So what's the most reasonable way to deal with this? I suppose we could 
rename virCommandRun to e.g. virCommandRunInternal() which would be 
called by a new virCommandRun(), then have virTestRewrapFile() call 
virCommandRunInternal() so that it wouldn't get the mocked version. That 
seems ugly, inefficient, and hackish, but I can't think of any way that 
isn't ugly, inefficient, and hackish...


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

Re: [libvirt] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

2016-04-12 Thread Jim Fehlig

Wei Liu wrote:
> Hi libvirt maintainers,

Sorry for the delay. Slowly catching up on mail after vacation...

> 
> Xen's control library libxenlight (libxl) requires application
> (libvirt in this case) to explictily define LIBXL_API_VERSION.

Where is this requirement written? :-)

> This is
> lacking at the moment so libvirt's libxl driver always gets the latest
> APIs.

IMO, that is what we want for upstream libvirt. Downstreams can choose a
specific version if they want.

> We changed one of the APIs in libxl so libvirt's libxl driver
> can't build at the moment.

A quick glance ahead in my libvirt mail tells me you fixed this with the
preferred LIBXL_HAVE_* pattern.

Regards,
Jim

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

Re: [libvirt] [PATCH] ZFS: Support sparse volumes

2016-04-12 Thread Richard Laager


On 03/16/2016 03:26 AM, Roman Bogorodskiy wrote:

Actually, the primary question is: do we want to add extra logic on top
of what ZFS does (i.e. disallow refres > volize, 'round' displayed
allocation) or just make things simple and do and show whatever ZFS
allows us to do and to show respectively.


I'm not necessarily sure I know the answer. So that probably argues for 
the simpler approach of libvirt just being a pass-through. Also, it's 
possible to add the extra code later, if that turns out to be better.


I've already submitted patches taking each approach. What do you think 
is the next step?


--
Richard

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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path



On 04/12/2016 03:55 PM, TomK wrote:
> 
> On 4/12/2016 3:40 PM, Martin Kletzander wrote:
>> [ I would be way easier to reply if you didn't top-post ]
>>
>> On Tue, Apr 12, 2016 at 12:07:50PM -0400, TomK wrote:
>>> Hey John,
>>>
>>> Hehe, I got the right guy then.  Very nice!  And very good ideas but I
>>> may need more time to reread and try them out later tonight. I'm fully
>>> in agreement about providing more details.  Can't be accurate in a
>>> diagnosis if there isn't much data to go on.  This pool option is new to
>>> me.  Please tell me more on it.  Can't find it in the file below but
>>> maybe it's elsewhere?
>>>
>>> (  ) perhaps rather than the "NFS" pool ( e.g. >> type="netfs"> )
>>>
>>>
>>> Allright, here's the details:
>>>
>>> [root@mdskvm-p01 ~]# rpm -aq|grep -i libvir
>>> libvirt-daemon-driver-secret-1.2.17-13.el7_2.4.x86_64
>>> libvirt-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-driver-network-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-driver-lxc-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-driver-nwfilter-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-driver-interface-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-config-network-1.2.17-13.el7_2.4.x86_64
>>> libvirt-client-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-driver-qemu-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-driver-storage-1.2.17-13.el7_2.4.x86_64
>>> libvirt-python-1.2.17-2.el7.x86_64
>>> libvirt-glib-0.1.9-1.el7.x86_64
>>> libvirt-daemon-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-config-nwfilter-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-driver-nodedev-1.2.17-13.el7_2.4.x86_64
>>> libvirt-daemon-kvm-1.2.17-13.el7_2.4.x86_64
>>> [root@mdskvm-p01 ~]# cat /etc/release
>>> cat: /etc/release: No such file or directory
>>> [root@mdskvm-p01 ~]# cat /etc/*release*
>>> NAME="Scientific Linux"
>>> VERSION="7.2 (Nitrogen)"
>>> ID="rhel"
>>> ID_LIKE="fedora"
>>> VERSION_ID="7.2"
>>> PRETTY_NAME="Scientific Linux 7.2 (Nitrogen)"
>>> ANSI_COLOR="0;31"
>>> CPE_NAME="cpe:/o:scientificlinux:scientificlinux:7.2:GA"
>>> HOME_URL="http://www.scientificlinux.org//;
>>> BUG_REPORT_URL="mailto:scientific-linux-de...@listserv.fnal.gov;
>>>
>>> REDHAT_BUGZILLA_PRODUCT="Scientific Linux 7"
>>> REDHAT_BUGZILLA_PRODUCT_VERSION=7.2
>>> REDHAT_SUPPORT_PRODUCT="Scientific Linux"
>>> REDHAT_SUPPORT_PRODUCT_VERSION="7.2"
>>> Scientific Linux release 7.2 (Nitrogen)
>>> Scientific Linux release 7.2 (Nitrogen)
>>> Scientific Linux release 7.2 (Nitrogen)
>>> cpe:/o:scientificlinux:scientificlinux:7.2:ga
>>> [root@mdskvm-p01 ~]#
>>>
>>> [root@mdskvm-p01 ~]# mount /var/lib/one
>>> [root@mdskvm-p01 ~]# su - oneadmin
>>> Last login: Sat Apr  9 10:39:25 EDT 2016 on pts/0
>>> Last failed login: Tue Apr 12 12:00:57 EDT 2016 from opennebula01 on
>>> ssh:notty
>>> There were 9584 failed login attempts since the last successful login.
>>> i[oneadmin@mdskvm-p01 ~]$ id oneadmin
>>> uid=9869(oneadmin) gid=9869(oneadmin)
>>> groups=9869(oneadmin),992(libvirt),36(kvm)
>>> [oneadmin@mdskvm-p01 ~]$ pwd
>>> /var/lib/one
>>> [oneadmin@mdskvm-p01 ~]$ ls -altriR|grep -i root
>>> 134320262 drwxr-xr-x. 45 root root4096 Apr 12 07:58 ..
>>> [oneadmin@mdskvm-p01 ~]$
>>>
>>>

It'd take more time than I have at the present moment to root out what
changed when for NFS root-squash, but suffice to say there were some
corner cases.  Some involving how qemu-img files are generated - I don't
have the details present in my short term memory.

>>>
>>> [oneadmin@mdskvm-p01 ~]$ cat /var/lib/one//datastores/0/38/deployment.0
>>> >> xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
>>> one-38
>>> 1
>>> 
>>> 1024
>>> 
>>> 524288
>>> 
>>> hvm
>>> 
>>> 
>>> 
>>> /usr/libexec/qemu-kvm
>>> 
>>> >> file='/var/lib/one//datastores/0/38/disk.0'/>
>>> 
>>> 
>>> 
>>> 
>>> >> file='/var/lib/one//datastores/0/38/disk.1'/>
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>>
>>> [oneadmin@mdskvm-p01 ~]$ cat
>>> /var/lib/one//datastores/0/38/deployment.0|grep -i nfs
>>> [oneadmin@mdskvm-p01 ~]$
>>>

Having/using a root squash via an NFS pool is "easy" (famous last words)

Create some pool XML (taking the example I have)

% cat nfs.xml

rootsquash






/tmp/netfs-rootsquash-pool

0755
107
107




In this case 107:107 is qemu:qemu and I used 'localhost' as the
hostname, but that can be a fqdn or ip-addr to the NFS server.

You've already seen my /etc/exports

virsh pool-define nfs.xml
virsh pool-build

Re: [libvirt] [libvirt-php PATCH 03/35] add missing arginfo (wip)

2016-04-12 Thread Neal Gompa

On Tue, Apr 12, 2016 at 11:13 AM, Michal Privoznik  wrote:
> This is not rebased onto current HEAD.
>
> Michal

Hello Michal,

I've rebased the patch set on the current HEAD as well as done some
patch squashing to reduce the number of patches[1]. Would you like for
me to resend the patch set based on top of the current HEAD?

[1]: https://gitlab.com/Conan_Kudo/libvirt-php7/compare/master...php7-review

-- 
真実はいつも一つ！/ Always, there's only one truth!

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

Re: [libvirt] [libvirt-php PATCH 09/35] adapt add_assoc_string_ex

2016-04-12 Thread Neal Gompa

On Tue, Apr 12, 2016 at 11:13 AM, Michal Privoznik  wrote:
>
> I'm afraid, this will not fly. While preprocessing this code, my
> compiler tries to expand RETURN_STRING() macro first (with VIRT_COPY_OPT
> still not expanded). Therefore it finds an argument missing and produces
> an compilation error.
> What we can do here is create a wrapper over RETURN_STRING, e.g.
> VIR_RETURN_STRING() that will behave differently for PHP7 and the rest.
>
> Unfortunately, this is where I have to stop the review as it's getting
> hairy and I gotta run. Sorry.
>
> Michal

Unfortunately, I cannot reproduce your problem. On my CentOS 7.2
system, I built, installed, and tested using gcc-4.8.5-4.el7 with
libvirt-1.2.17-13.el7_2.4 and its associated devel package. I've
compiled it as a PHP 5.4 module (using the default PHP), as a PHP 5.6
module (using Remi's SCL for PHP 5.6), and as a PHP 7.0 module (using
Remi's SCL for PHP 7.0). It worked properly with both build systems,
and all 17 tests passed with the `make test` run on the newer build
system.

For reference on my CentOS 7.2 system, I'm using the following:
* gcc-4.8.5-4.el7
* libvirt-daemon-1.2.17-13.el7_2.4
* libvirt-devel-1.2.17-13.el7_2.4
* xz-devel-5.1.2-12alpha.el7
* libxml2-devel-2.9.1-6.el7_2.2
* libxslt-1.1.28-5.el7
* xhtml1-dtds-1.0-20020801.11.el7

Here are the PHP packages I have installed:
* php-cli-5.4.16-36.el7_1
* php-common-5.4.16-36.el7_1
* php-devel-5.4.16-36.el7_1
* php56-runtime-2.1-5.el7.remi
* php56-php-cli-5.6.20-1.el7.remi
* php56-php-pecl-jsonc-1.3.9-1.el7.remi
* php56-php-pecl-zip-1.13.2-1.el7.remi
* php56-php-pecl-jsonc-devel-1.3.9-1.el7.remi
* php56-php-common-5.6.20-1.el7.remi
* php56-php-devel-5.6.20-1.el7.remi
* php70-runtime-1.0-4.el7.remi
* php70-php-json-7.0.5-1.el7.remi
* php70-php-cli-7.0.5-1.el7.remi
* php70-php-common-7.0.5-1.el7.remi
* php70-php-devel-7.0.5-1.el7.remi


For kicks, I tested on Fedora 23 with the latest system libvirt and
php 5.6 packages available, and everything seems to work as expected.

Here are the packages on my Fedora system:
* gcc-5.3.1-2.fc23
* libvirt-daemon-1.2.18.2-3.fc23
* libvirt-devel-1.2.18.2-3.fc23
* xz-devel-5.2.1-3.fc23
* libxml2-devel-2.9.3-2.fc23
* libxslt-1.1.28-11.fc23
* xhtml1-dtds-1.0-20020801.13.fc23

Here are the PHP packages on my Fedora system:
* php-cli-5.6.20-1.fc23
* php-common-5.6.20-1.fc23
* php-pecl-jsonc-1.3.9-1.fc23
* php-pecl-jsonc-devel-1.3.9-1.fc23
* php-devel-5.6.20-1.fc23

Obviously there's some kind of mismatch in my environment and yours
that is causing problems. Could you please tell me what your
environment is and what you're using so that I can see if I can
reproduce the issue?

-- 
真実はいつも一つ！/ Always, there's only one truth!

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

[libvirt] [PATCH] qemu: migration: Drop dead VNC cookie handling

The only caller of this code is:

for (i = 0; i < dom->def->ngraphics; i++) {
   if (dom->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
   if (!(mig->graphics =
 qemuMigrationCookieGraphicsAlloc(driver, 
dom->def->graphics[i])))
   return -1;
   mig->flags |= QEMU_MIGRATION_COOKIE_GRAPHICS;
   break;
   }
}

So this is never triggered for VNC, and in fact VNC has no support for
seamless migration anyways so that seems correct. Drop the dead VNC
handling.
---
 src/qemu/qemu_migration.c | 34 +++---
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8d2ca3b..ca3619d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -320,33 +320,21 @@ qemuMigrationCookieGraphicsAlloc(virQEMUDriverPtr driver,
 goto error;
 
 mig->type = def->type;
-if (mig->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
-mig->port = def->data.vnc.port;
-listenAddr = virDomainGraphicsListenGetAddress(def, 0);
-if (!listenAddr)
-listenAddr = cfg->vncListen;
+mig->port = def->data.spice.port;
+if (cfg->spiceTLS)
+mig->tlsPort = def->data.spice.tlsPort;
+else
+mig->tlsPort = -1;
+listenAddr = virDomainGraphicsListenGetAddress(def, 0);
+if (!listenAddr)
+listenAddr = cfg->spiceListen;
 
 #ifdef WITH_GNUTLS
-if (cfg->vncTLS &&
-!(mig->tlsSubject = 
qemuDomainExtractTLSSubject(cfg->vncTLSx509certdir)))
-goto error;
+if (cfg->spiceTLS &&
+!(mig->tlsSubject = 
qemuDomainExtractTLSSubject(cfg->spiceTLSx509certdir)))
+goto error;
 #endif
-} else {
-mig->port = def->data.spice.port;
-if (cfg->spiceTLS)
-mig->tlsPort = def->data.spice.tlsPort;
-else
-mig->tlsPort = -1;
-listenAddr = virDomainGraphicsListenGetAddress(def, 0);
-if (!listenAddr)
-listenAddr = cfg->spiceListen;
 
-#ifdef WITH_GNUTLS
-if (cfg->spiceTLS &&
-!(mig->tlsSubject = 
qemuDomainExtractTLSSubject(cfg->spiceTLSx509certdir)))
-goto error;
-#endif
-}
 if (VIR_STRDUP(mig->listen, listenAddr) < 0)
 goto error;
 
-- 
2.7.3

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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path


On Tue, Apr 12, 2016 at 10:29:29PM +0200, Martin Kletzander wrote:

On Tue, Apr 12, 2016 at 03:55:45PM -0400, TomK wrote:

On 4/12/2016 3:40 PM, Martin Kletzander wrote:

[ I would be way easier to reply if you didn't top-post ]

On Tue, Apr 12, 2016 at 12:07:50PM -0400, TomK wrote:

On 4/12/2016 11:45 AM, John Ferlan wrote:

What got my attention was the error message "initializing FS storage
file" with the "file:" prefix to the name and 9869:9869 as the uid:gid
trying to access the file (I assume that's oneadmin:oneadmin on your
system).



I totally missed this.  So the only thing that popped on my mind now was
checking the whole path:

 ls -ld /var{,/lib{,/one{,/datastores{,/0{,/38{,/disk.1}}

You can also run it as root and oneadmin, however after reading through
all the info again, I don't think that'll help.


I top post by default in thunderbird and we have same setup at work with
M$ LookOut.  Old habits are to blame I guess.  I'll try to reply  like
this instead.  But yeah it's terrible for mailing lists to top post.
Here's the output and thanks again:

[oneadmin@mdskvm-p01 ~]$ ls -ld
/var{,/lib{,/one{,/datastores{,/0{,/38{,/disk.1}}
drwxr-xr-x. 21 root root   4096 Apr 11 07:10 /var
drwxr-xr-x. 45 root root   4096 Apr 12 07:58 /var/lib
drwxr-x---  12 oneadmin oneadmin   4096 Apr 12 15:50 /var/lib/one


Look ^^, maybe for a quick workaround you could try doing:

 chmod o+rx /var/lib/one



Actually, o+x ought to be enough.


Let me know if that does the trick (at least for now).


drwxrwxr-x   6 oneadmin oneadmin 46 Mar 31 02:44 /var/lib/one/datastores
drwxrwxr-x   6 oneadmin oneadmin 42 Apr  5 00:20
/var/lib/one/datastores/0
drwxrwxr-x   2 oneadmin oneadmin 68 Apr  5 00:20
/var/lib/one/datastores/0/38
-rw-r--r--   1 oneadmin oneadmin 372736 Apr  5 00:20
/var/lib/one/datastores/0/38/disk.1
[oneadmin@mdskvm-p01 ~]$

That's the default setting but I think I see what you're getting at that
permissions get inherited?



No, I just think you need eXecute on all parent directories.  That
shouldn't hinder your security and could help.


Cheers,
Tom K.
-


Living on earth is expensive, but it includes a free trip around the sun.






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


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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path


On Tue, Apr 12, 2016 at 03:55:45PM -0400, TomK wrote:

On 4/12/2016 3:40 PM, Martin Kletzander wrote:

[ I would be way easier to reply if you didn't top-post ]

On Tue, Apr 12, 2016 at 12:07:50PM -0400, TomK wrote:

On 4/12/2016 11:45 AM, John Ferlan wrote:

What got my attention was the error message "initializing FS storage
file" with the "file:" prefix to the name and 9869:9869 as the uid:gid
trying to access the file (I assume that's oneadmin:oneadmin on your
system).



I totally missed this.  So the only thing that popped on my mind now was
checking the whole path:

 ls -ld /var{,/lib{,/one{,/datastores{,/0{,/38{,/disk.1}}

You can also run it as root and oneadmin, however after reading through
all the info again, I don't think that'll help.


I top post by default in thunderbird and we have same setup at work with
M$ LookOut.  Old habits are to blame I guess.  I'll try to reply  like
this instead.  But yeah it's terrible for mailing lists to top post.
Here's the output and thanks again:

[oneadmin@mdskvm-p01 ~]$ ls -ld
/var{,/lib{,/one{,/datastores{,/0{,/38{,/disk.1}}
drwxr-xr-x. 21 root root   4096 Apr 11 07:10 /var
drwxr-xr-x. 45 root root   4096 Apr 12 07:58 /var/lib
drwxr-x---  12 oneadmin oneadmin   4096 Apr 12 15:50 /var/lib/one


Look ^^, maybe for a quick workaround you could try doing:

 chmod o+rx /var/lib/one

Let me know if that does the trick (at least for now).


drwxrwxr-x   6 oneadmin oneadmin 46 Mar 31 02:44 /var/lib/one/datastores
drwxrwxr-x   6 oneadmin oneadmin 42 Apr  5 00:20
/var/lib/one/datastores/0
drwxrwxr-x   2 oneadmin oneadmin 68 Apr  5 00:20
/var/lib/one/datastores/0/38
-rw-r--r--   1 oneadmin oneadmin 372736 Apr  5 00:20
/var/lib/one/datastores/0/38/disk.1
[oneadmin@mdskvm-p01 ~]$

That's the default setting but I think I see what you're getting at that
permissions get inherited?



No, I just think you need eXecute on all parent directories.  That
shouldn't hinder your security and could help.


Cheers,
Tom K.
-


Living on earth is expensive, but it includes a free trip around the sun.



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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path



On 4/12/2016 3:40 PM, Martin Kletzander wrote:

[ I would be way easier to reply if you didn't top-post ]

On Tue, Apr 12, 2016 at 12:07:50PM -0400, TomK wrote:

Hey John,

Hehe, I got the right guy then.  Very nice!  And very good ideas but I
may need more time to reread and try them out later tonight. I'm fully
in agreement about providing more details.  Can't be accurate in a
diagnosis if there isn't much data to go on.  This pool option is new to
me.  Please tell me more on it.  Can't find it in the file below but
maybe it's elsewhere?

(  ) perhaps rather than the "NFS" pool ( e.g. type="netfs"> )



Allright, here's the details:

[root@mdskvm-p01 ~]# rpm -aq|grep -i libvir
libvirt-daemon-driver-secret-1.2.17-13.el7_2.4.x86_64
libvirt-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-network-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-lxc-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-interface-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-network-1.2.17-13.el7_2.4.x86_64
libvirt-client-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-qemu-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-storage-1.2.17-13.el7_2.4.x86_64
libvirt-python-1.2.17-2.el7.x86_64
libvirt-glib-0.1.9-1.el7.x86_64
libvirt-daemon-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nodedev-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-kvm-1.2.17-13.el7_2.4.x86_64
[root@mdskvm-p01 ~]# cat /etc/release
cat: /etc/release: No such file or directory
[root@mdskvm-p01 ~]# cat /etc/*release*
NAME="Scientific Linux"
VERSION="7.2 (Nitrogen)"
ID="rhel"
ID_LIKE="fedora"
VERSION_ID="7.2"
PRETTY_NAME="Scientific Linux 7.2 (Nitrogen)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:scientificlinux:scientificlinux:7.2:GA"
HOME_URL="http://www.scientificlinux.org//;
BUG_REPORT_URL="mailto:scientific-linux-de...@listserv.fnal.gov;

REDHAT_BUGZILLA_PRODUCT="Scientific Linux 7"
REDHAT_BUGZILLA_PRODUCT_VERSION=7.2
REDHAT_SUPPORT_PRODUCT="Scientific Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="7.2"
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
cpe:/o:scientificlinux:scientificlinux:7.2:ga
[root@mdskvm-p01 ~]#

[root@mdskvm-p01 ~]# mount /var/lib/one
[root@mdskvm-p01 ~]# su - oneadmin
Last login: Sat Apr  9 10:39:25 EDT 2016 on pts/0
Last failed login: Tue Apr 12 12:00:57 EDT 2016 from opennebula01 on
ssh:notty
There were 9584 failed login attempts since the last successful login.
i[oneadmin@mdskvm-p01 ~]$ id oneadmin
uid=9869(oneadmin) gid=9869(oneadmin)
groups=9869(oneadmin),992(libvirt),36(kvm)
[oneadmin@mdskvm-p01 ~]$ pwd
/var/lib/one
[oneadmin@mdskvm-p01 ~]$ ls -altriR|grep -i root
134320262 drwxr-xr-x. 45 root root4096 Apr 12 07:58 ..
[oneadmin@mdskvm-p01 ~]$



[oneadmin@mdskvm-p01 ~]$ cat /var/lib/one//datastores/0/38/deployment.0
xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>

one-38
1

1024

524288

hvm



/usr/libexec/qemu-kvm






















[oneadmin@mdskvm-p01 ~]$ cat
/var/lib/one//datastores/0/38/deployment.0|grep -i nfs
[oneadmin@mdskvm-p01 ~]$



Cheers,
Tom K.
- 



Living on earth is expensive, but it includes a free trip around the 
sun.


On 4/12/2016 11:45 AM, John Ferlan wrote:


On 04/12/2016 10:58 AM, TomK wrote:

Hey Martin,

Thanks very much.  Appreciate you jumping in on this thread.

Can you provide some more details with respect to which libvirt version
you have installed. I know I've made changes in this space in more
recent versions (not the most recent). I'm no root_squash expert, but I
was the last to change things in the space so that makes me partially
fluent ;-) in NFS/root_squash speak.



I'm always lost in how do we handle *all* the corner cases that are not
even used anywhere at all, but care about the conditions we have in the
code.  Especially when it's constantly changing.  So thanks for jumping
in.  I only replied because nobody else did and I had only the tiniest
clue as to what could happen.


Using root_squash is very "finicky" (to say the least)... It wasn't
really clear from what you posted how you are attempting to reference
things. Does the "/var/lib/one//datastores/0/38/deployment.0" XML file
use a direct path to the NFS volume or does it use a pool? If a pool,
then what type of pool? It is beneficial to provide as many details as
possible about the configuration

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path


[ I would be way easier to reply if you didn't top-post ]

On Tue, Apr 12, 2016 at 12:07:50PM -0400, TomK wrote:

Hey John,

Hehe, I got the right guy then.  Very nice!  And very good ideas but I
may need more time to reread and try them out later tonight.  I'm fully
in agreement about providing more details.  Can't be accurate in a
diagnosis if there isn't much data to go on.  This pool option is new to
me.  Please tell me more on it.  Can't find it in the file below but
maybe it's elsewhere?

(  ) perhaps rather than the "NFS" pool ( e.g.  )


Allright, here's the details:

[root@mdskvm-p01 ~]# rpm -aq|grep -i libvir
libvirt-daemon-driver-secret-1.2.17-13.el7_2.4.x86_64
libvirt-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-network-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-lxc-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-interface-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-network-1.2.17-13.el7_2.4.x86_64
libvirt-client-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-qemu-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-storage-1.2.17-13.el7_2.4.x86_64
libvirt-python-1.2.17-2.el7.x86_64
libvirt-glib-0.1.9-1.el7.x86_64
libvirt-daemon-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nodedev-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-kvm-1.2.17-13.el7_2.4.x86_64
[root@mdskvm-p01 ~]# cat /etc/release
cat: /etc/release: No such file or directory
[root@mdskvm-p01 ~]# cat /etc/*release*
NAME="Scientific Linux"
VERSION="7.2 (Nitrogen)"
ID="rhel"
ID_LIKE="fedora"
VERSION_ID="7.2"
PRETTY_NAME="Scientific Linux 7.2 (Nitrogen)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:scientificlinux:scientificlinux:7.2:GA"
HOME_URL="http://www.scientificlinux.org//;
BUG_REPORT_URL="mailto:scientific-linux-de...@listserv.fnal.gov;

REDHAT_BUGZILLA_PRODUCT="Scientific Linux 7"
REDHAT_BUGZILLA_PRODUCT_VERSION=7.2
REDHAT_SUPPORT_PRODUCT="Scientific Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="7.2"
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
cpe:/o:scientificlinux:scientificlinux:7.2:ga
[root@mdskvm-p01 ~]#

[root@mdskvm-p01 ~]# mount /var/lib/one
[root@mdskvm-p01 ~]# su - oneadmin
Last login: Sat Apr  9 10:39:25 EDT 2016 on pts/0
Last failed login: Tue Apr 12 12:00:57 EDT 2016 from opennebula01 on
ssh:notty
There were 9584 failed login attempts since the last successful login.
i[oneadmin@mdskvm-p01 ~]$ id oneadmin
uid=9869(oneadmin) gid=9869(oneadmin)
groups=9869(oneadmin),992(libvirt),36(kvm)
[oneadmin@mdskvm-p01 ~]$ pwd
/var/lib/one
[oneadmin@mdskvm-p01 ~]$ ls -altriR|grep -i root
134320262 drwxr-xr-x. 45 root root4096 Apr 12 07:58 ..
[oneadmin@mdskvm-p01 ~]$



[oneadmin@mdskvm-p01 ~]$ cat /var/lib/one//datastores/0/38/deployment.0

one-38
1

1024

524288

hvm



/usr/libexec/qemu-kvm






















[oneadmin@mdskvm-p01 ~]$ cat
/var/lib/one//datastores/0/38/deployment.0|grep -i nfs
[oneadmin@mdskvm-p01 ~]$



Cheers,
Tom K.
-

Living on earth is expensive, but it includes a free trip around the sun.

On 4/12/2016 11:45 AM, John Ferlan wrote:


On 04/12/2016 10:58 AM, TomK wrote:

Hey Martin,

Thanks very much.  Appreciate you jumping in on this thread.

Can you provide some more details with respect to which libvirt version
you have installed. I know I've made changes in this space in more
recent versions (not the most recent). I'm no root_squash expert, but I
was the last to change things in the space so that makes me partially
fluent ;-) in NFS/root_squash speak.



I'm always lost in how do we handle *all* the corner cases that are not
even used anywhere at all, but care about the conditions we have in the
code.  Especially when it's constantly changing.  So thanks for jumping
in.  I only replied because nobody else did and I had only the tiniest
clue as to what could happen.


Using root_squash is very "finicky" (to say the least)... It wasn't
really clear from what you posted how you are attempting to reference
things. Does the "/var/lib/one//datastores/0/38/deployment.0" XML file
use a direct path to the NFS volume or does it use a pool? If a pool,
then what type of pool? It is beneficial to provide as many details as
possible about the configuration because (so to speak) those that are
helping you won't know your environment (I've never used OpenNebula) nor
do I have a

[libvirt] [PATCH v3 2/6] vz: introduce new vzDriver lockable structure and use it

This patch introduces a new 'vzDriver' lockable object and provides
helper functions to allocate/destroy it and we pass it to prlsdkXxx
functions instead of virConnectPtr.
Now we store domain related objects such as domain list, capabitilies
etc. within a single vz_driver vzDriver structure, which is shared by
all driver connections. It is allocated during daemon initialization or
in a lazy manner when a new connection to 'vz' driver is established.
When a connection to vz daemon drops, vzDestroyConnection is called,
which in turn relays disconnect event to all connection to 'vz' driver.

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_driver.c | 334 +++--
 src/vz/vz_sdk.c| 209 -
 src/vz/vz_sdk.h|  30 ++---
 src/vz/vz_utils.c  |  27 ++---
 src/vz/vz_utils.h  |  31 +++--
 5 files changed, 375 insertions(+), 256 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 1f4c380..7564b6a 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -63,19 +63,27 @@ VIR_LOG_INIT("parallels.parallels_driver");
 #define PRLCTL  "prlctl"
 
 static int vzConnectClose(virConnectPtr conn);
+static virClassPtr vzDriverClass;
 
 void
-vzDriverLock(vzConnPtr driver)
+vzDriverLock(vzConnPtr privconn)
 {
-virMutexLock(>lock);
+virObjectLock(privconn->driver);
 }
 
 void
-vzDriverUnlock(vzConnPtr driver)
+vzDriverUnlock(vzConnPtr privconn)
 {
-virMutexUnlock(>lock);
+virObjectUnlock(privconn->driver);
 }
 
+static virMutex vz_driver_lock;
+static vzDriverPtr vz_driver;
+static vzConnPtr vz_conn_list;
+
+static vzDriverPtr
+vzDriverObjNew(void);
+
 static int
 vzCapsAddGuestDomain(virCapsPtr caps,
  virDomainOSType ostype,
@@ -158,6 +166,70 @@ vzBuildCapabilities(void)
 goto cleanup;
 }
 
+static void vzDriverDispose(void * obj)
+{
+vzDriverPtr driver = obj;
+
+if (driver->server) {
+prlsdkUnsubscribeFromPCSEvents(driver);
+prlsdkDisconnect(driver);
+}
+
+virObjectUnref(driver->domains);
+virObjectUnref(driver->caps);
+virObjectUnref(driver->xmlopt);
+virObjectEventStateFree(driver->domainEventState);
+}
+
+static int vzDriverOnceInit(void)
+{
+if (!(vzDriverClass = virClassNew(virClassForObjectLockable(),
+  "vzDriver",
+  sizeof(vzDriver),
+  vzDriverDispose)))
+return -1;
+
+return 0;
+}
+VIR_ONCE_GLOBAL_INIT(vzDriver)
+
+vzDriverPtr
+vzGetDriverConnection(void)
+{
+virMutexLock(_driver_lock);
+if (!vz_driver)
+vz_driver = vzDriverObjNew();
+virObjectRef(vz_driver);
+virMutexUnlock(_driver_lock);
+
+return vz_driver;
+}
+
+void
+vzDestroyDriverConnection(void)
+{
+
+vzDriverPtr driver;
+vzConnPtr privconn_list;
+
+virMutexLock(_driver_lock);
+driver = vz_driver;
+vz_driver = NULL;
+
+privconn_list = vz_conn_list;
+vz_conn_list = NULL;
+
+virMutexUnlock(_driver_lock);
+
+while (privconn_list) {
+vzConnPtr privconn = privconn_list;
+privconn_list = privconn->next;
+virConnectCloseCallbackDataCall(privconn->closeCallback,
+VIR_CONNECT_CLOSE_REASON_EOF);
+}
+virObjectUnref(driver);
+}
+
 static char *
 vzConnectGetCapabilities(virConnectPtr conn)
 {
@@ -165,7 +237,7 @@ vzConnectGetCapabilities(virConnectPtr conn)
 char *xml;
 
 vzDriverLock(privconn);
-xml = virCapabilitiesFormatXML(privconn->caps);
+xml = virCapabilitiesFormatXML(privconn->driver->caps);
 vzDriverUnlock(privconn);
 return xml;
 }
@@ -214,70 +286,34 @@ virDomainDefParserConfig vzDomainDefParserConfig = {
 .domainPostParseCallback = vzDomainDefPostParse,
 };
 
-
-static int
-vzOpenDefault(virConnectPtr conn)
+static vzDriverPtr
+vzDriverObjNew(void)
 {
-vzConnPtr privconn;
+vzDriverPtr driver;
 
-if (VIR_ALLOC(privconn) < 0)
-return VIR_DRV_OPEN_ERROR;
-if (virMutexInit(>lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("cannot initialize mutex"));
-goto err_free;
-}
-
-if (prlsdkInit()) {
-VIR_DEBUG("%s", _("Can't initialize Parallels SDK"));
-goto err_free;
-}
-
-if (prlsdkConnect(privconn) < 0)
-goto err_free;
-
-if (vzInitVersion(privconn) < 0)
-goto error;
-
-if (!(privconn->caps = vzBuildCapabilities()))
-goto error;
-
-vzDomainDefParserConfig.priv = >vzCaps;
-if (!(privconn->xmlopt = virDomainXMLOptionNew(,
-   NULL, NULL)))
-goto error;
-
-if (!(privconn->domains = virDomainObjListNew()))
-goto error;
-
-if (!(privconn->domainEventState = virObjectEventStateNew()))
-goto error;
-
-if

Re: [libvirt] [PATCH 01/10] util: Rename and move virStrIsPrint to virStringIsPrintable

2016-04-12 Thread Jiri Denemark

On Tue, Apr 12, 2016 at 15:55:35 +0200, Peter Krempa wrote:
> ---
>  src/conf/domain_conf.c   |  4 ++--
>  src/libvirt_private.syms |  2 +-
>  src/util/virstring.c | 18 ++
>  src/util/virstring.h |  2 ++
>  src/util/virutil.c   | 12 
>  src/util/virutil.h   |  2 --
>  6 files changed, 23 insertions(+), 17 deletions(-)
...
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 2d7fbf3..384e3f7 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -1048,3 +1048,21 @@ virStringToUpper(char **dst, const char *src)
>  *dst = cap;
>  return 1;
>  }
> +
> +
> +/**
> + * virStrIsPrintable:

virStringIsPrintable

Jirka

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

[libvirt] [PATCH v3 3/6] vz: lock driver when a new domain is created in prlsdkNewDomainByHandle

there can be a race with vzDomainDefineXMLFlags

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_sdk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index db14e78..dcc2703 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1257,6 +1257,7 @@ prlsdkNewDomainByHandle(vzDriverPtr driver, PRL_HANDLE 
sdkdom)
 if (prlsdkGetDomainIds(sdkdom, , uuid) < 0)
 goto cleanup;
 
+virObjectLock(driver);
 if (!(dom = vzNewDomain(driver, name, uuid)))
 goto cleanup;
 
@@ -1267,6 +1268,7 @@ prlsdkNewDomainByHandle(vzDriverPtr driver, PRL_HANDLE 
sdkdom)
 }
 
  cleanup:
+virObjectUnlock(driver);
 VIR_FREE(name);
 return dom;
 }
-- 
2.4.3

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

[libvirt] [PATCH v3 4/6] vz: implement connectGetSysinfo hypervisor callback

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_driver.c | 25 +
 src/vz/vz_utils.h  |  1 +
 2 files changed, 26 insertions(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 7564b6a..03b3aa8 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -179,6 +179,7 @@ static void vzDriverDispose(void * obj)
 virObjectUnref(driver->caps);
 virObjectUnref(driver->xmlopt);
 virObjectEventStateFree(driver->domainEventState);
+virSysinfoDefFree(driver->hostsysinfo);
 }
 
 static int vzDriverOnceInit(void)
@@ -312,6 +313,7 @@ vzDriverObjNew(void)
 return NULL;
 }
 
+driver->hostsysinfo = virSysinfoRead();
 ignore_value(prlsdkLoadDomains(driver));
 return driver;
 }
@@ -421,6 +423,28 @@ static char *vzConnectGetHostname(virConnectPtr conn 
ATTRIBUTE_UNUSED)
 return virGetHostname();
 }
 
+static char *
+vzConnectGetSysinfo(virConnectPtr conn, unsigned int flags)
+{
+vzConnPtr privconn = conn->privateData;
+vzDriverPtr driver = privconn->driver;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virCheckFlags(0, NULL);
+
+if (!driver->hostsysinfo) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Host SMBIOS information is not available"));
+return NULL;
+}
+
+if (virSysinfoFormat(, driver->hostsysinfo) < 0)
+return NULL;
+if (virBufferCheckError() < 0)
+return NULL;
+
+return virBufferContentAndReset();
+}
 
 static int
 vzConnectListDomains(virConnectPtr conn, int *ids, int maxids)
@@ -1576,6 +1600,7 @@ static virHypervisorDriver vzHypervisorDriver = {
 .connectClose = vzConnectClose,  /* 0.10.0 */
 .connectGetVersion = vzConnectGetVersion,   /* 0.10.0 */
 .connectGetHostname = vzConnectGetHostname,  /* 0.10.0 */
+.connectGetSysinfo = vzConnectGetSysinfo, /* 1.3.4 */
 .connectGetMaxVcpus = vzConnectGetMaxVcpus, /* 1.2.21 */
 .nodeGetInfo = vzNodeGetInfo,  /* 0.10.0 */
 .nodeGetCPUStats = vzNodeGetCPUStats,  /* 1.2.21 */
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index 5c14048..64a0348 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -70,6 +70,7 @@ struct _vzDriver {
 virCapsPtr caps;
 virDomainXMLOptionPtr xmlopt;
 virObjectEventStatePtr domainEventState;
+virSysinfoDefPtr hostsysinfo;
 unsigned long vzVersion;
 vzCapabilities vzCaps;
 };
-- 
2.4.3

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

[libvirt] [PATCH v3 1/6] vz: build driver as module and don't register it on client's side

Make it possible to build vz driver as a module and don't link it with
libvirt.so statically.
Remove registering it on client's side as far as we start relying on daemon

Signed-off-by: Maxim Nestratov 
---
 daemon/Makefile.am   |  4 
 daemon/libvirtd.c|  9 +
 src/Makefile.am  | 19 ++-
 src/libvirt.c|  7 ---
 src/libvirt_private.syms |  7 +++
 5 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 2dbe81b..78d7d21 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -233,6 +233,10 @@ if WITH_VBOX
 libvirtd_LDADD += ../src/libvirt_driver_vbox.la
 endif WITH_VBOX
 
+if WITH_VZ
+libvirtd_LDADD += ../src/libvirt_driver_vz.la
+endif WITH_VZ
+
 if WITH_STORAGE
 libvirtd_LDADD += ../src/libvirt_driver_storage.la
 endif WITH_STORAGE
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 3d38a46..92b4080 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -102,6 +102,9 @@
 #  include "nwfilter/nwfilter_driver.h"
 # endif
 #endif
+#ifdef WITH_VZ
+# include "vz/vz_driver.h"
+#endif
 
 #include "configmake.h"
 
@@ -390,6 +393,9 @@ static void daemonInitialize(void)
 # ifdef WITH_BHYVE
 virDriverLoadModule("bhyve");
 # endif
+# ifdef WITH_VZ
+virDriverLoadModule("vz");
+# endif
 #else
 # ifdef WITH_NETWORK
 networkRegister();
@@ -430,6 +436,9 @@ static void daemonInitialize(void)
 # ifdef WITH_BHYVE
 bhyveRegister();
 # endif
+# ifdef WITH_VZ
+vzRegister();
+# endif
 #endif
 }
 
diff --git a/src/Makefile.am b/src/Makefile.am
index eda0365..08ff301 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -630,6 +630,7 @@ DRIVER_SOURCE_FILES = \
$(NULL)
 
 STATEFUL_DRIVER_SOURCE_FILES = \
+   $(VZ_DRIVER_SOURCES) \
$(BHYVE_DRIVER_SOURCES) \
$(INTERFACE_DRIVER_SOURCES) \
$(LIBXL_DRIVER_SOURCES) \
@@ -886,7 +887,7 @@ HYPERV_DRIVER_EXTRA_DIST =  
\
hyperv/hyperv_wmi_generator.py  
\
$(HYPERV_DRIVER_GENERATED)
 
-VZ_DRIVER_SOURCES =\
+VZ_DRIVER_SOURCES =\
vz/vz_driver.h  \
vz/vz_driver.c  \
vz/vz_utils.c   \
@@ -1493,13 +1494,21 @@ libvirt_driver_hyperv_la_SOURCES = 
$(HYPERV_DRIVER_SOURCES)
 endif WITH_HYPERV
 
 if WITH_VZ
+noinst_LTLIBRARIES += libvirt_driver_vz_impl.la
+libvirt_driver_vz_la_SOURCES =
+libvirt_driver_vz_la_LIBADD = libvirt_driver_vz_impl.la
+if WITH_DRIVER_MODULES
+mod_LTLIBRARIES += libvirt_driver_vz.la
+libvirt_driver_vz_la_LIBADD += ../gnulib/lib/libgnu.la
+libvirt_driver_vz_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
+else ! WITH_DRIVER_MODULES
 noinst_LTLIBRARIES += libvirt_driver_vz.la
-libvirt_la_BUILT_LIBADD += libvirt_driver_vz.la
-libvirt_driver_vz_la_CFLAGS = \
+endif ! WITH_DRIVER_MODULES
+libvirt_driver_vz_impl_la_CFLAGS = \
-I$(srcdir)/conf $(AM_CFLAGS) \
$(PARALLELS_SDK_CFLAGS) $(LIBNL_CFLAGS)
-libvirt_driver_vz_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS)
-libvirt_driver_vz_la_SOURCES = $(VZ_DRIVER_SOURCES)
+libvirt_driver_vz_impl_la_SOURCES = $(VZ_DRIVER_SOURCES)
+libvirt_driver_vz_impl_la_LIBADD =  $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS)
 endif WITH_VZ
 
 if WITH_BHYVE
diff --git a/src/libvirt.c b/src/libvirt.c
index dd58e9c..a21d00e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -92,9 +92,6 @@
 #ifdef WITH_XENAPI
 # include "xenapi/xenapi_driver.h"
 #endif
-#ifdef WITH_VZ
-# include "vz/vz_driver.h"
-#endif
 #ifdef WITH_BHYVE
 # include "bhyve/bhyve_driver.h"
 #endif
@@ -433,10 +430,6 @@ virGlobalInit(void)
 if (xenapiRegister() == -1)
 goto error;
 # endif
-# ifdef WITH_VZ
-if (vzRegister() == -1)
-goto error;
-# endif
 #endif
 #ifdef WITH_REMOTE
 if (remoteRegister() == -1)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 068bc00..5319711 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -931,6 +931,11 @@ virGetSecret;
 virGetStoragePool;
 virGetStorageVol;
 virGetStream;
+virConnectCloseCallbackDataGetCallback;
+virNewConnectCloseCallbackData;
+virConnectCloseCallbackDataUnregister;
+virConnectCloseCallbackDataRegister;
+virConnectCloseCallbackDataCall;
 virInterfaceClass;
 virNetworkClass;
 virNodeDeviceClass;
@@ -939,6 +944,7 @@ virSecretClass;
 virStoragePoolClass;
 virStorageVolClass;
 virStreamClass;
+virConnectCloseCallbackDataClass;
 
 
 # fdstream.h
@@ -1306,6 +1312,7 @@ virCommandHandshakeWait;
 virCommandNew;
 virCommandNewArgList;
 virCommandNewArgs;
+virCommandNewVAList;
 virCommandNonblockingFDs;
 virCommandPassFD;
 virCommandPassFDGetFDIndex;
-- 
2.4.3

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

[libvirt] [PATCH v3 6/6] vz: minor cleanup

remove unnecessary vzConnectClose prototype and make
local structure vzDomainDefParserConfig be static

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_driver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 81fb130..ce50919 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -62,7 +62,6 @@ VIR_LOG_INIT("parallels.parallels_driver");
 
 #define PRLCTL  "prlctl"
 
-static int vzConnectClose(virConnectPtr conn);
 static virClassPtr vzDriverClass;
 
 static virMutex vz_driver_lock;
@@ -267,7 +266,7 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 }
 
 
-virDomainDefParserConfig vzDomainDefParserConfig = {
+static virDomainDefParserConfig vzDomainDefParserConfig = {
 .macPrefix = {0x42, 0x1C, 0x00},
 .devicesPostParseCallback = vzDomainDeviceDefPostParse,
 .domainPostParseCallback = vzDomainDefPostParse,
-- 
2.4.3

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

[libvirt] [PATCH v3 0/6] vz: change vz driver to be stateful driver and other enhancements

There is no benefit in providing two ways of connecting to vz driver:
by connecting via daemon and directly from client. Both ways finally
come to a host where vz daemon sits. Always connecting via daemon allows
us to have a single list of domains and share it among all connections.

Since v1:
 removed patch "z: remove close callback implementations"
 building fixed
 close callback functions are added to libvirt_private.syms
 reworked not to lose event subscribers when connections drop

Since v2:
 removed "vz: change vzConnectIsAlive behavior"
 addressed mostly all comments on previous series
 changed "vz: build driver as module and don't register it on client's side"

Maxim Nestratov (6):
  vz: build driver as module and don't register it on client's side
  vz: introduce new vzDriver lockable structure and use it
  vz: lock driver when a new domain is created in
prlsdkNewDomainByHandle
  vz: implement connectGetSysinfo hypervisor callback
  vz: remove vzDriverLock/Unlock function
  vz: minor cleanup

 daemon/Makefile.am   |   4 +
 daemon/libvirtd.c|   9 ++
 src/Makefile.am  |  19 ++-
 src/libvirt.c|   7 -
 src/libvirt_private.syms |   7 +
 src/vz/vz_driver.c   | 384 +--
 src/vz/vz_sdk.c  | 211 +-
 src/vz/vz_sdk.h  |  30 ++--
 src/vz/vz_utils.c|  27 ++--
 src/vz/vz_utils.h|  34 +++--
 10 files changed, 431 insertions(+), 301 deletions(-)

-- 
2.4.3

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

[libvirt] [PATCH v3 5/6] vz: remove vzDriverLock/Unlock function

We don't need them anymore as all pointers within vzDriver structure
are not changed during the time it exists.
Where we still need to synchronize we use virObjectLock/Unlock as far
as vzDriver is lockable object.

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_driver.c | 40 
 src/vz/vz_utils.h  |  2 --
 2 files changed, 4 insertions(+), 38 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 03b3aa8..81fb130 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -65,18 +65,6 @@ VIR_LOG_INIT("parallels.parallels_driver");
 static int vzConnectClose(virConnectPtr conn);
 static virClassPtr vzDriverClass;
 
-void
-vzDriverLock(vzConnPtr privconn)
-{
-virObjectLock(privconn->driver);
-}
-
-void
-vzDriverUnlock(vzConnPtr privconn)
-{
-virObjectUnlock(privconn->driver);
-}
-
 static virMutex vz_driver_lock;
 static vzDriverPtr vz_driver;
 static vzConnPtr vz_conn_list;
@@ -237,9 +225,7 @@ vzConnectGetCapabilities(virConnectPtr conn)
 vzConnPtr privconn = conn->privateData;
 char *xml;
 
-vzDriverLock(privconn);
 xml = virCapabilitiesFormatXML(privconn->driver->caps);
-vzDriverUnlock(privconn);
 return xml;
 }
 
@@ -452,10 +438,8 @@ vzConnectListDomains(virConnectPtr conn, int *ids, int 
maxids)
 vzConnPtr privconn = conn->privateData;
 int n;
 
-vzDriverLock(privconn);
 n = virDomainObjListGetActiveIDs(privconn->driver->domains, ids, maxids,
  NULL, NULL);
-vzDriverUnlock(privconn);
 
 return n;
 }
@@ -466,10 +450,8 @@ vzConnectNumOfDomains(virConnectPtr conn)
 vzConnPtr privconn = conn->privateData;
 int count;
 
-vzDriverLock(privconn);
 count = virDomainObjListNumOfDomains(privconn->driver->domains, true,
  NULL, NULL);
-vzDriverUnlock(privconn);
 
 return count;
 }
@@ -480,11 +462,9 @@ vzConnectListDefinedDomains(virConnectPtr conn, char 
**const names, int maxnames
 vzConnPtr privconn = conn->privateData;
 int n;
 
-vzDriverLock(privconn);
 memset(names, 0, sizeof(*names) * maxnames);
 n = virDomainObjListGetInactiveNames(privconn->driver->domains, names,
  maxnames, NULL, NULL);
-vzDriverUnlock(privconn);
 
 return n;
 }
@@ -495,11 +475,8 @@ vzConnectNumOfDefinedDomains(virConnectPtr conn)
 vzConnPtr privconn = conn->privateData;
 int count;
 
-vzDriverLock(privconn);
 count = virDomainObjListNumOfDomains(privconn->driver->domains, false,
  NULL, NULL);
-vzDriverUnlock(privconn);
-
 return count;
 }
 
@@ -512,10 +489,8 @@ vzConnectListAllDomains(virConnectPtr conn,
 int ret = -1;
 
 virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
-vzDriverLock(privconn);
 ret = virDomainObjListExport(privconn->driver->domains, conn, domains,
  NULL, flags);
-vzDriverUnlock(privconn);
 
 return ret;
 }
@@ -527,9 +502,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
 virDomainPtr ret = NULL;
 virDomainObjPtr dom;
 
-vzDriverLock(privconn);
 dom = virDomainObjListFindByID(privconn->driver->domains, id);
-vzDriverUnlock(privconn);
 
 if (dom == NULL) {
 virReportError(VIR_ERR_NO_DOMAIN, NULL);
@@ -553,10 +526,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 virDomainPtr ret = NULL;
 virDomainObjPtr dom;
 
-vzDriverLock(privconn);
-
 dom = virDomainObjListFindByUUID(privconn->driver->domains, uuid);
-vzDriverUnlock(privconn);
 
 if (dom == NULL) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -583,9 +553,7 @@ vzDomainLookupByName(virConnectPtr conn, const char *name)
 virDomainPtr ret = NULL;
 virDomainObjPtr dom;
 
-vzDriverLock(privconn);
 dom = virDomainObjListFindByName(privconn->driver->domains, name);
-vzDriverUnlock(privconn);
 
 if (dom == NULL) {
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -1520,7 +1488,7 @@ vzConnectRegisterCloseCallback(virConnectPtr conn,
 vzConnPtr privconn = conn->privateData;
 int ret = -1;
 
-vzDriverLock(privconn);
+virObjectLock(privconn->driver);
 if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != 
NULL) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("A close callback is already registered"));
@@ -1532,7 +1500,7 @@ vzConnectRegisterCloseCallback(virConnectPtr conn,
 ret = 0;
 
  cleanup:
-vzDriverUnlock(privconn);
+virObjectUnlock(privconn->driver);
 
 return ret;
 }
@@ -1543,8 +1511,8 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, 
virConnectCloseFunc cb)
 vzConnPtr privconn = conn->privateData;
 int ret = -1;
 
-vzDriverLock(privconn);
 
+virObjectLock(privconn->driver);
 if

Re: [libvirt] [PATCH v2 06/10] vz: introduce new vzDriver lockable structure and use it


12.04.2016 14:11, Nikolay Shirokovskiy пишет:


On 07.04.2016 23:09, Maxim Nestratov wrote:

This patch introduces a new 'vzDriver' lockable object and provides
helper functions to allocate/destroy it and we pass it to prlsdkXxx
functions instead of virConnectPtr.
Now we store domain related objects such as domain list, capabitilies
etc. within a single vz_driver vzDriver structure, which is shared by
all driver connections. It is allocated during daemon initialization or
in a lazy manner when a new connection to 'vz' driver is established.
When a connection to vz daemon drops, vzDestroyConnection is called,
which in turn relays disconnect event to all connection to 'vz' driver.

Signed-off-by: Maxim Nestratov 
---
  src/vz/vz_driver.c | 339 +++--
  src/vz/vz_sdk.c| 211 -
  src/vz/vz_sdk.h|  30 ++---
  src/vz/vz_utils.c  |  27 +++--
  src/vz/vz_utils.h  |  29 -
  5 files changed, 380 insertions(+), 256 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index f2bbf1e..e9fe89f 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -63,18 +63,25 @@ VIR_LOG_INIT("parallels.parallels_driver");
  #define PRLCTL  "prlctl"
  
  static int vzConnectClose(virConnectPtr conn);

+static virClassPtr vzDriverConnClass;

why 'conn' suffix? i suggest just vzDriverClass


fixed



  
  void

-vzDriverLock(vzConnPtr driver)
+vzDriverLock(vzConnPtr privconn)
  {
-virMutexLock(>lock);
+virObjectLock(privconn->driver);
  }
  
  void

-vzDriverUnlock(vzConnPtr driver)
+vzDriverUnlock(vzConnPtr privconn)
  {
-virMutexUnlock(>lock);
+virObjectUnlock(privconn->driver);
  }

empty line here will be good


agree


+static virMutex vz_driver_lock;
+static vzDriverPtr vz_driver;
+static vzConnPtr vz_conn_list;

i would move this list into driver,
i even think if we could write version of close callback object
that can take multiple callbacks, thus we can get rid of vz_conn_list
and vzConn altogether.


not sure about moving
as for a new version of close callback - certainly not in this patch series


+
+static vzDriverPtr
+vzDriverObjNew(void);
  
  static int

  vzCapsAddGuestDomain(virCapsPtr caps,
@@ -158,6 +165,69 @@ vzBuildCapabilities(void)
  goto cleanup;
  }
  
+static void vzDriverDispose(void * obj)

+{
+vzDriverPtr conn = obj;

looks like 'driver' will be better here


agree




+
+if (conn->server) {
+prlsdkUnsubscribeFromPCSEvents(conn);
+prlsdkDisconnect(conn);
+}
+
+virObjectUnref(conn->domains);
+virObjectUnref(conn->caps);
+virObjectUnref(conn->xmlopt);
+virObjectEventStateFree(conn->domainEventState);
+}
+
+static int vzDriverOnceInit(void)
+{
+if (!(vzDriverConnClass = virClassNew(virClassForObjectLockable(),
+"vzDriver",
+sizeof(vzDriver),
+vzDriverDispose)))

indentation


ahh, sure


+return -1;
+
+return 0;
+}
+VIR_ONCE_GLOBAL_INIT(vzDriver)
+
+vzDriverPtr
+vzGetDriverConnection(void)
+{
+virMutexLock(_driver_lock);
+if (!vz_driver)
+vz_driver = vzDriverObjNew();
+virObjectRef(vz_driver);
+virMutexUnlock(_driver_lock);
+return vz_driver;
+}

i would put more empty lines here,


not sure, but if you insist...


and since this func is pretty simple i suggest open code it in that new function


didn't get you, sorry




+
+void
+vzDestroyDriverConnection(void)
+{
+
+vzDriverPtr driver;
+vzConnPtr privconn_list;
+
+virMutexLock(_driver_lock);
+driver = vz_driver;
+vz_driver = NULL;
+
+privconn_list = vz_conn_list;
+vz_conn_list = NULL;
+
+virMutexUnlock(_driver_lock);
+
+while (privconn_list) {
+vzConnPtr privconn = privconn_list;
+privconn_list = privconn->next;
+virConnectCloseCallbackDataCall(privconn->closeCallback,
+VIR_CONNECT_CLOSE_REASON_EOF);
+}
+virObjectUnref(driver);
+}
+
  static char *
  vzConnectGetCapabilities(virConnectPtr conn)
  {
@@ -165,7 +235,7 @@ vzConnectGetCapabilities(virConnectPtr conn)
  char *xml;
  
  vzDriverLock(privconn);

-xml = virCapabilitiesFormatXML(privconn->caps);
+xml = virCapabilitiesFormatXML(privconn->driver->caps);
  vzDriverUnlock(privconn);
  return xml;
  }
@@ -214,70 +284,34 @@ virDomainDefParserConfig vzDomainDefParserConfig = {
  .domainPostParseCallback = vzDomainDefPostParse,
  };
  
-

-static int
-vzOpenDefault(virConnectPtr conn)
+static vzDriverPtr
+vzDriverObjNew(void)
  {
-vzConnPtr privconn;
-
-if (VIR_ALLOC(privconn) < 0)
-return VIR_DRV_OPEN_ERROR;
-if (virMutexInit(>lock) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("cannot initialize mutex"));
-goto err_free;
-}
+

Re: [libvirt] [PATCH v2 04/10] vz: add Hypervisor prefix to vz and parallels Driver structures


12.04.2016 17:04, Nikolay Shirokovskiy пишет:


On 07.04.2016 23:09, Maxim Nestratov wrote:

---
  src/vz/vz_driver.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 9de88cd..f2bbf1e 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1491,7 +1491,7 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, 
virConnectCloseFunc cb)
  return ret;
  }
  
-static virHypervisorDriver vzDriver = {

+static virHypervisorDriver vzHypervisorDriver = {
  .name = "vz",
  .connectOpen = vzConnectOpen,/* 0.10.0 */
  .connectClose = vzConnectClose,  /* 0.10.0 */
@@ -1558,11 +1558,11 @@ static virHypervisorDriver vzDriver = {
  };
  
  static virConnectDriver vzConnectDriver = {

-.hypervisorDriver = ,
+.hypervisorDriver = ,
  };
  
  /* Parallels domain type backward compatibility*/

-static virHypervisorDriver parallelsDriver;
+static virHypervisorDriver parallelsHypervisorDriver;
  static virConnectDriver parallelsConnectDriver;
  
  /**

@@ -1584,10 +1584,10 @@ vzRegister(void)
  VIR_FREE(prlctl_path);
  
  /* Backward compatibility with Parallels domain type */

-parallelsDriver = vzDriver;
-parallelsDriver.name = "Parallels";
+parallelsHypervisorDriver = vzHypervisorDriver;
+parallelsHypervisorDriver.name = "Parallels";
  parallelsConnectDriver = vzConnectDriver;
-parallelsConnectDriver.hypervisorDriver = 
+parallelsConnectDriver.hypervisorDriver = 
  if (virRegisterConnectDriver(, false) < 0)
  return -1;
  


ACK





Thanks.
Pushed 1-4 of the series.

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

Re: [libvirt] [PATCH 0/2] host-validate: Don't build on Windows

2016-04-12 Thread Andrea Bolognani

On Fri, 2016-04-08 at 14:03 -0400, Cole Robinson wrote:
> On 04/08/2016 12:04 PM, Andrea Bolognani wrote:
> > 
> > The recent fixes to virt-host-validate have caused the mingw
> > build to fail[1].
> > 
> > Instead of working around Windows' quirks in the tool, just
> > stop building it altogether - it's not like it made any
> > sense to have it available on that OS anyway.
> > 
> > Cheers.
> > 
> > 
> > [1] https://ci.centos.org/view/libvirt-project/job/libvirt-mingw/1496/
> > 
> > Andrea Bolognani (2):
> >   tools: Reorganize conditional bits
> >   configure: Make virt-host-validate optional
> > 
> >  configure.ac | 29 ++---
> >  m4/virt-host-validate.m4 | 40 
> >  tools/Makefile.am| 35 ++-
> >  3 files changed, 76 insertions(+), 28 deletions(-)
> >  create mode 100644 m4/virt-host-validate.m4
> 
> I applied them both, tried 'make distcheck', and hit:
> 
>  /usr/bin/install -c -m 644 ../../../tools/virt-host-validate.1
> ../../../tools/virt-pki-validate.1 ../../../tools/virt-xml-validate.1
> ../../../tools/virsh.1 ../../../tools/virt-admin.1
> ../../../tools/virt-login-shell.1 ../../../tools/virt-host-validate.1
> '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1'
> /usr/bin/install: will not overwrite just-created
> '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1/virt-host-validate.1'
> with '../../../tools/virt-host-validate.1'
> Makefile:2863: recipe for target 'install-man1' failed
> make[4]: *** [install-man1] Error 1
> make[4]: *** Waiting for unfinished jobs
> 
> Makefiles suck :)

I just realized Martin didn't include you when replying.

Would you mind trying again after squashing in the hunk
from[1]? That should fix the failure you're seeing.

Cheers.


[1] https://www.redhat.com/archives/libvir-list/2016-April/msg00485.html
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

[libvirt] [PATCH v2] util: Add virGettextInitialize, convert the code

Take setlocale/gettext error handling pattern from tools/virsh-*
and use it for all standalone binaries via a new shared
virGettextInitialize routine. The virsh* pattern differed slightly
from other callers. All users now consistently:

* Ignore setlocale errors. virsh has done this forever, presumably for
  good reason. This has been partially responsible for some bug reports:

  https://bugzilla.redhat.com/show_bug.cgi?id=1312688
  https://bugzilla.redhat.com/show_bug.cgi?id=1026514
  https://bugzilla.redhat.com/show_bug.cgi?id=1016158

* Report the failed function name
* Report strerror
---
v2:
Add virGettextInitialize rather than duplicate the code everywhere

 daemon/libvirtd.c |  6 ++---
 src/Makefile.am   |  2 ++
 src/libvirt_private.syms  |  4 
 src/locking/lock_daemon.c |  6 ++---
 src/locking/sanlock_helper.c  |  9 ++-
 src/logging/log_daemon.c  |  6 ++---
 src/lxc/lxc_controller.c  |  6 ++---
 src/network/leaseshelper.c| 12 +++---
 src/security/virt-aa-helper.c | 12 +++---
 src/storage/parthelper.c  |  9 ++-
 src/util/iohelper.c   | 13 +++---
 src/util/virgettext.c | 56 +++
 src/util/virgettext.h | 25 +++
 tools/virsh.c | 15 ++--
 tools/virt-admin.c| 15 ++--
 tools/virt-host-validate.c| 15 ++--
 tools/virt-login-shell.c  | 14 ++-
 tools/vsh.c   |  2 --
 18 files changed, 116 insertions(+), 111 deletions(-)
 create mode 100644 src/util/virgettext.c
 create mode 100644 src/util/virgettext.h

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 3d38a46..5f66e8b 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -30,7 +30,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include "libvirt_internal.h"
 #include "virerror.h"
@@ -58,6 +57,7 @@
 #include "locking/lock_manager.h"
 #include "viraccessmanager.h"
 #include "virutil.h"
+#include "virgettext.h"
 
 #ifdef WITH_DRIVER_MODULES
 # include "driver.h"
@@ -1172,9 +1172,7 @@ int main(int argc, char **argv) {
 {0, 0, 0, 0}
 };
 
-if (setlocale(LC_ALL, "") == NULL ||
-bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-textdomain(PACKAGE) == NULL ||
+if (virGettextInitialize() < 0 ||
 virInitialize() < 0) {
 fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
 exit(EXIT_FAILURE);
diff --git a/src/Makefile.am b/src/Makefile.am
index eda0365..38b9560 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -114,6 +114,7 @@ UTIL_SOURCES =  
\
util/virfile.c util/virfile.h   \
util/virfirewall.c util/virfirewall.h   \
util/virfirewallpriv.h  \
+   util/virgettext.c util/virgettext.h \
util/virgic.c util/virgic.h \
util/virhash.c util/virhash.h   \
util/virhashcode.c util/virhashcode.h   \
@@ -2321,6 +2322,7 @@ libvirt_setuid_rpc_client_la_SOURCES =\
util/virevent.c \
util/vireventpoll.c \
util/virfile.c  \
+   util/virgettext.c   \
util/virhash.c  \
util/virhashcode.c  \
util/virjson.c  \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 068bc00..af7de8a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1517,6 +1517,10 @@ virFirewallStartRollback;
 virFirewallStartTransaction;
 
 
+# util/virgettext.h
+virGettextInitialize;
+
+
 # util/virgic.h
 virGICVersionTypeFromString;
 virGICVersionTypeToString;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 973e691..bfdcfc6 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -28,7 +28,6 @@
 #include 
 #include 
 #include 
-#include 
 
 
 #include "lock_daemon.h"
@@ -47,6 +46,7 @@
 #include "virhash.h"
 #include "viruuid.h"
 #include "virstring.h"
+#include "virgettext.h"
 
 #include "locking/lock_daemon_dispatch.h"
 #include "locking/lock_protocol.h"
@@ -1179,9 +1179,7 @@ int main(int argc, char **argv) {
 
 privileged = geteuid() == 0;
 
-if (setlocale(LC_ALL, "") == NULL ||
-bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-textdomain(PACKAGE) == NULL ||
+if (virGettextInitialize() < 0 ||
 virThreadInitialize() < 0 ||
 virErrorInitialize() < 0) {
 fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
diff --git a/src/locking/sanlock_helper.c b/src/locking/sanlock_helper.c
index d8d294f..57e1cfb 100644
--- a/src/locking/sanlock_helper.c
+++ b/src/locking/sanlock_helper.c
@@ -1,13 +1,12 @@

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path


Hey John,

Hehe, I got the right guy then.  Very nice!  And very good ideas but I 
may need more time to reread and try them out later tonight.  I'm fully 
in agreement about providing more details.  Can't be accurate in a 
diagnosis if there isn't much data to go on.  This pool option is new to 
me.  Please tell me more on it.  Can't find it in the file below but 
maybe it's elsewhere?


(  ) perhaps rather than the "NFS" pool ( e.g.  )


Allright, here's the details:

[root@mdskvm-p01 ~]# rpm -aq|grep -i libvir
libvirt-daemon-driver-secret-1.2.17-13.el7_2.4.x86_64
libvirt-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-network-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-lxc-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-interface-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-network-1.2.17-13.el7_2.4.x86_64
libvirt-client-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-qemu-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-storage-1.2.17-13.el7_2.4.x86_64
libvirt-python-1.2.17-2.el7.x86_64
libvirt-glib-0.1.9-1.el7.x86_64
libvirt-daemon-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-config-nwfilter-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-driver-nodedev-1.2.17-13.el7_2.4.x86_64
libvirt-daemon-kvm-1.2.17-13.el7_2.4.x86_64
[root@mdskvm-p01 ~]# cat /etc/release
cat: /etc/release: No such file or directory
[root@mdskvm-p01 ~]# cat /etc/*release*
NAME="Scientific Linux"
VERSION="7.2 (Nitrogen)"
ID="rhel"
ID_LIKE="fedora"
VERSION_ID="7.2"
PRETTY_NAME="Scientific Linux 7.2 (Nitrogen)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:scientificlinux:scientificlinux:7.2:GA"
HOME_URL="http://www.scientificlinux.org//;
BUG_REPORT_URL="mailto:scientific-linux-de...@listserv.fnal.gov;

REDHAT_BUGZILLA_PRODUCT="Scientific Linux 7"
REDHAT_BUGZILLA_PRODUCT_VERSION=7.2
REDHAT_SUPPORT_PRODUCT="Scientific Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="7.2"
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
Scientific Linux release 7.2 (Nitrogen)
cpe:/o:scientificlinux:scientificlinux:7.2:ga
[root@mdskvm-p01 ~]#

[root@mdskvm-p01 ~]# mount /var/lib/one
[root@mdskvm-p01 ~]# su - oneadmin
Last login: Sat Apr  9 10:39:25 EDT 2016 on pts/0
Last failed login: Tue Apr 12 12:00:57 EDT 2016 from opennebula01 on 
ssh:notty

There were 9584 failed login attempts since the last successful login.
i[oneadmin@mdskvm-p01 ~]$ id oneadmin
uid=9869(oneadmin) gid=9869(oneadmin) 
groups=9869(oneadmin),992(libvirt),36(kvm)

[oneadmin@mdskvm-p01 ~]$ pwd
/var/lib/one
[oneadmin@mdskvm-p01 ~]$ ls -altriR|grep -i root
134320262 drwxr-xr-x. 45 root root4096 Apr 12 07:58 ..
[oneadmin@mdskvm-p01 ~]$



[oneadmin@mdskvm-p01 ~]$ cat /var/lib/one//datastores/0/38/deployment.0

one-38
1

1024

524288

hvm



/usr/libexec/qemu-kvm

file='/var/lib/one//datastores/0/38/disk.0'/>





file='/var/lib/one//datastores/0/38/disk.1'/>
















[oneadmin@mdskvm-p01 ~]$ cat 
/var/lib/one//datastores/0/38/deployment.0|grep -i nfs

[oneadmin@mdskvm-p01 ~]$



Cheers,
Tom K.
- 


Living on earth is expensive, but it includes a free trip around the sun.

On 4/12/2016 11:45 AM, John Ferlan wrote:


On 04/12/2016 10:58 AM, TomK wrote:

Hey Martin,

Thanks very much.  Appreciate you jumping in on this thread.

Can you provide some more details with respect to which libvirt version
you have installed. I know I've made changes in this space in more
recent versions (not the most recent). I'm no root_squash expert, but I
was the last to change things in the space so that makes me partially
fluent ;-) in NFS/root_squash speak.

Using root_squash is very "finicky" (to say the least)... It wasn't
really clear from what you posted how you are attempting to reference
things. Does the "/var/lib/one//datastores/0/38/deployment.0" XML file
use a direct path to the NFS volume or does it use a pool? If a pool,
then what type of pool? It is beneficial to provide as many details as
possible about the configuration because (so to speak) those that are
helping you won't know your environment (I've never used OpenNebula) nor
do I have a 'oneadmin' uid:gid.

What got my attention was the error message "initializing FS storage
file" with the "file:" prefix to the name and 9869:9869 as the uid:gid
trying to access the file (I assume that's oneadmin:oneadmin on your
system).

This says to me that you're trying to use a "file system" pool (e.g
) perhaps rather than the

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path

On 04/12/2016 10:58 AM, TomK wrote:
> Hey Martin,
> 
> Thanks very much.  Appreciate you jumping in on this thread.

Can you provide some more details with respect to which libvirt version
you have installed. I know I've made changes in this space in more
recent versions (not the most recent). I'm no root_squash expert, but I
was the last to change things in the space so that makes me partially
fluent ;-) in NFS/root_squash speak.

Using root_squash is very "finicky" (to say the least)... It wasn't
really clear from what you posted how you are attempting to reference
things. Does the "/var/lib/one//datastores/0/38/deployment.0" XML file
use a direct path to the NFS volume or does it use a pool? If a pool,
then what type of pool? It is beneficial to provide as many details as
possible about the configuration because (so to speak) those that are
helping you won't know your environment (I've never used OpenNebula) nor
do I have a 'oneadmin' uid:gid.

What got my attention was the error message "initializing FS storage
file" with the "file:" prefix to the name and 9869:9869 as the uid:gid
trying to access the file (I assume that's oneadmin:oneadmin on your
system).

This says to me that you're trying to use a "file system" pool (e.g
) perhaps rather than the "NFS" pool (e.g. ). Using an NFS pool certainly has the advantage of
"knowing how" to deal with the NFS environment. Since libvirt may
consider this to "just" be a FS file, then it won't necessarily know to
try to access the file properly (OK dependent upon libvirt version too
perhaps - the details have been paged out of my memory while I do other
work).

One other thing that popped out at me:

My /etc/exports has:

/home/bzs/rootsquash/nfs *(rw,sync,root_squash)

which only differs from yours by the 'no_subtree_check'

your environment though seems to have much more "depth" than mine. That
is you have "//datastores/0/38/disk.1" appended on as the (I assume)
disk to use. The question then becomes - does every directory in the
path to that file use "oneadmin:oneadmin" and of course does it have to
with[out] that extra flag.

Again, I'm no expert just trying to provide ideas and help...

John

> 
> You see, that's just it.  I've configured libvirt .conf files to run as
> oneadmin.oneadmin (non previlidged) for that NFS share and I can access
> all the files on that share as oneadmin without error, including the one
> you listed.  But libvirtd, by default, always starts as root.  So it's
> doing something as root, despite being configured to access the share as
> oneadmin.  As oneadmin I can access that file no problem.  Here's how I
> read the file off the node on which the NFS share is mounted on:
> 
> [oneadmin@mdskvm-p01 ~]$ ls -altri /var/lib/one//datastores/0/38/disk.1
> 34642274 -rw-r--r-- 1 oneadmin oneadmin 372736 Apr 5 00:20
> /var/lib/one//datastores/0/38/disk.1
> [oneadmin@mdskvm-p01 ~]$ file /var/lib/one//datastores/0/38/disk.1
> /var/lib/one//datastores/0/38/disk.1: # ISO 9660 CD-ROM filesystem data
> 'CONTEXT'
> [oneadmin@mdskvm-p01 ~]$ strings /var/lib/one//datastores/0/38/disk.1|head
> CD001
> LINUX CONTEXT
> GENISOIMAGE ISO 9660/HFS FILESYSTEM CREATOR (C) 1993 E.YOUNGDALE (C)
> 1997-2006 J.PEARSON/J.SCHILLING (C) 2006-2007 CDRKIT TEAM 2016040500205600
> 2016040500205600
> 
> 2016040500205600
> 
> CD001
> 2016040500205600
> 2016040500205600
> [oneadmin@mdskvm-p01 ~]$
> 
> My NFS mount looks as follows ( I have to use root_squash for security
> reasons.  I'm sure it will work using no_root_squash but that option is
> not an option here.):
> 
> [root@mdskvm-p01 ~]# grep nfs /etc/fstab
> # 192.168.0.70:/var/lib/one//var/lib/one/  nfs  
> context=system_u:object_r:nfs_t:s0,soft,intr,rsize=8192,wsize=8192,noauto
> 192.168.0.70:/var/lib/one/  /var/lib/one/  nfs  
> soft,intr,rsize=8192,wsize=8192,noauto
> [root@mdskvm-p01 ~]#
> 
> [root@opennebula01 ~]# cat /etc/exports
> /var/lib/one/ *(rw,sync,no_subtree_check,root_squash)
> [root@opennebula01 ~]#
> 
> 
> So I dug deeper and see that there is a possibility libvirtd is trying
> to access that NFS mount as root as some level because as root I also
> get a permission denied to the NFS share above.  Rightly so since I have
> root_squash that I need to keep.  But libvirtd should be able to access
> the file as oneadmin as I have above.  It's not and this is what I read
> on it:
> 
> https://www.redhat.com/archives/libvir-list/2014-May/msg00194.html
> 
> Comment is: "The current implementation works for local
> storage only and returns the canonical path of the volume."
> 
> But it seems the logic is applied to NFS mounts. Perhaps it shouldn't
> be?  Anyway to get around this problem?  This is CentOS 7 .
> 
> My post with OpenNebula is here from which this conversation originates:
> https://forum.opennebula.org/t/libvirtd-running-as-root-tries-to-access-oneadmin-nfs-mount-error-cant-canonicalize-path/2054/7
> 
> Cheers,
> Tom K.
>

Re: [libvirt] [libvirt-php PATCH 03/35] add missing arginfo (wip)

On 09.04.2016 00:08, Neal Gompa wrote:
> From: Remi Collet 
> 
> ---
>  src/libvirt-php.c | 458 
> +++---
>  1 file changed, 371 insertions(+), 87 deletions(-)
> 

This is not rebased onto current HEAD.

Michal

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

Re: [libvirt] [libvirt-php PATCH 09/35] adapt add_assoc_string_ex

On 09.04.2016 00:08, Neal Gompa wrote:
> From: Remi Collet 
> 
> ---
>  src/libvirt-php.c | 299 
> ++
>  1 file changed, 99 insertions(+), 200 deletions(-)
> 
> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index d37fd6f..0459bb9 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c
> @@ -58,6 +58,7 @@ const char *features_binaries[] = { NULL };
>  #if PHP_MAJOR_VERSION >= 7
>  typedef size_t strsize_t;
>  
> +#define VIRT_COPY_OPT
>  #define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
>   if ((_state = (_type)zend_fetch_resource(Z_RES_P(*_zval), _name, _le)) 
> == NULL) { \
>   RETURN_FALSE; \
> @@ -68,6 +69,8 @@ typedef int strsize_t;
>  typedef long zend_long;
>  typedef unsigned long zend_ulong;
>  
> +#define VIRT_COPY_OPT ,1
> +
>  #define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
>   ZEND_FETCH_RESOURCE(_state, _type, _zval, -1, _name, _le);
>  
> @@ -1995,7 +1998,7 @@ if ((snapshot==NULL) || (snapshot->snapshot==NULL)) 
> RETURN_FALSE;\
>  #define LONGLONG_ASSOC(out,key,in) \
>  if (LIBVIRT_G(longlong_to_string_ini)) { \
>  snprintf(tmpnumber,63,"%llu",in); \
> -add_assoc_string_ex(out,key,strlen(key)+1,tmpnumber,1); \
> +add_assoc_string_ex(out,key,strlen(key)+1,tmpnumber VIRT_COPY_OPT); \
>  } \
>  else \
>  { \
> @@ -2059,11 +2062,7 @@ static int libvirt_virConnectCredType[] = {
>  PHP_FUNCTION(libvirt_get_last_error)
>  {
>  if (LIBVIRT_G (last_error) == NULL) RETURN_NULL();
> -#if PHP_MAJOR_VERSION >= 7
> -RETURN_STRING(LIBVIRT_G (last_error));
> -#else
> -RETURN_STRING(LIBVIRT_G (last_error),1);
> -#endif
> +RETURN_STRING(LIBVIRT_G (last_error) VIRT_COPY_OPT);

I'm afraid, this will not fly. While preprocessing this code, my
compiler tries to expand RETURN_STRING() macro first (with VIRT_COPY_OPT
still not expanded). Therefore it finds an argument missing and produces
an compilation error.
What we can do here is create a wrapper over RETURN_STRING, e.g.
VIR_RETURN_STRING() that will behave differently for PHP7 and the rest.

Unfortunately, this is where I have to stop the review as it's getting
hairy and I gotta run. Sorry.

Michal

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

Re: [libvirt] [libvirt-php PATCH 04/35] fix typo in config.m4

On 09.04.2016 00:08, Neal Gompa wrote:
> From: Remi Collet 
> 
> ---
>  install-sh| 366 
> +++---
>  missing   |   2 +-
>  src/config.m4 |  12 +-
>  3 files changed, 204 insertions(+), 176 deletions(-)

This looks very suspicious. Why is install-sh and missing updated then?
Moreover, this should have been merged into 01/35. You just don't send a
broken patch in a series followed by its fix in the very same patchset.
But this should not be directed at you, Neal.

Michal

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

Re: [libvirt] [libvirt-php PATCH 01/35] add config.m4 to allow standard PHP extension build system

On 09.04.2016 00:08, Neal Gompa wrote:
> From: Remi Collet 
> 
> ---
>  src/config.m4 | 52 
>  1 file changed, 52 insertions(+)
>  create mode 100644 src/config.m4
> 
> diff --git a/src/config.m4 b/src/config.m4
> new file mode 100644
> index 000..ee2b47d
> --- /dev/null
> +++ b/src/config.m4
> @@ -0,0 +1,52 @@
> +PHP_ARG_WITH(libvirt, for libvirt support,
> +[  --with-libvirt Include varnish support])

What's varnish? It should have been libvirt instead.

> +
> +if test "$PHP_LIBVIRT" != "no"; then
> +
> +  AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
> +
> +  if test -x "$PKG_CONFIG" && $PKG_CONFIG varnishapi --exists ; then
> +AC_MSG_CHECKING(libvirt version)
> +if $PKG_CONFIG libvirt --atleast-version=1.2.8 ; then
> +  LIBVIRT_INCLUDE=`$PKG_CONFIG libvirt --cflags`
> +  LIBVIRT_LIBRARY=`$PKG_CONFIG libvirt --libs`
> +  LIBVIRT_VERSION=`$PKG_CONFIG libvirt --modversion`
> +  AC_MSG_RESULT($LIBVIRT_VERSION)
> +else
> +  AC_MSG_ERROR(version too old)
> +fi
> +PHP_EVAL_INCLINE($LIBVIRT_INCLUDE)
> +PHP_EVAL_LIBLINE($LIBVIRT_LIBRARY, LIBVIRT_SHARED_LIBADD)
> +
> +AC_MSG_CHECKING(libvirt-qemu version)
> +if $PKG_CONFIG libvirt-qemu --atleast-version=1.2.8 ; then
> +  QEMU_INCLUDE=`$PKG_CONFIG libvirt-qemu --cflags`
> +  QEMU_LIBRARY=`$PKG_CONFIG libvirt-qemu --libs`
> +  QEMU_VERSION=`$PKG_CONFIG libvirt-qemu --modversion`
> +  AC_MSG_RESULT($QEMU_VERSION)
> +else
> +  AC_MSG_ERROR(version too old)
> +fi
> +PHP_EVAL_INCLINE($QEMU_INCLUDE)
> +PHP_EVAL_LIBLINE($QEMU_LIBRARY, LIBVIRT_SHARED_LIBADD)
> +
> +AC_MSG_CHECKING(libxml version)
> +if $PKG_CONFIG libxml-2.0 --atleast-version=1.2.8 ; then
> +  LIBXML_INCLUDE=`$PKG_CONFIG libxml-2.0 --cflags`
> +  LIBXML_LIBRARY=`$PKG_CONFIG libxml-2.0 --libs`
> +  LIBXML_VERSION=`$PKG_CONFIG libxml-2.0 --modversion`
> +  AC_MSG_RESULT($LIBXML_VERSION)
> +else
> +  AC_MSG_ERROR(version too old)
> +fi
> +PHP_EVAL_INCLINE($LIBXML_INCLUDE)
> +PHP_EVAL_LIBLINE($LIBXML_LIBRARY, LIBVIRT_SHARED_LIBADD)
> +
> +CFLAGS="$CFLAGS -DCOMPILE_DL_LIBVIRT=1"
> +
> +PHP_SUBST(LIBVIRT_SHARED_LIBADD)
> +PHP_NEW_EXTENSION(libvirt, libvirt-php.c sockets.c vncfunc.c, 
> $ext_shared)
> +  else
> +AC_MSG_ERROR(pkg-config not found)
> +  fi
> +fi
> 

Michal

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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path

Hey Martin,

Thanks very much. Appreciate you jumping in on this thread.

You see, that's just it. I've configured libvirt .conf files to run as
oneadmin.oneadmin (non previlidged) for that NFS share and I can access
all the files on that share as oneadmin without error, including the one
you listed. But libvirtd, by default, always starts as root. So it's
doing something as root, despite being configured to access the share as
oneadmin. As oneadmin I can access that file no problem. Here's how I
read the file off the node on which the NFS share is mounted on:

[oneadmin@mdskvm-p01 ~]$ ls -altri /var/lib/one//datastores/0/38/disk.1
34642274 -rw-r--r-- 1 oneadmin oneadmin 372736 Apr 5 00:20
/var/lib/one//datastores/0/38/disk.1

[oneadmin@mdskvm-p01 ~]$ file /var/lib/one//datastores/0/38/disk.1
/var/lib/one//datastores/0/38/disk.1: # ISO 9660 CD-ROM filesystem data
'CONTEXT'

[oneadmin@mdskvm-p01 ~]$ strings /var/lib/one//datastores/0/38/disk.1|head
CD001
LINUX CONTEXT
GENISOIMAGE ISO 9660/HFS FILESYSTEM CREATOR (C) 1993 E.YOUNGDALE (C)
1997-2006 J.PEARSON/J.SCHILLING (C) 2006-2007 CDRKIT TEAM 2016040500205600

2016040500205600

CD001
2016040500205600
2016040500205600
[oneadmin@mdskvm-p01 ~]$

My NFS mount looks as follows ( I have to use root_squash for security
reasons. I'm sure it will work using no_root_squash but that option is
not an option here.):

[root@mdskvm-p01 ~]# grep nfs /etc/fstab
# 192.168.0.70:/var/lib/one//var/lib/one/ nfs
context=system_u:object_r:nfs_t:s0,soft,intr,rsize=8192,wsize=8192,noauto
192.168.0.70:/var/lib/one/ /var/lib/one/ nfs
soft,intr,rsize=8192,wsize=8192,noauto

[root@mdskvm-p01 ~]#

[root@opennebula01 ~]# cat /etc/exports
/var/lib/one/ *(rw,sync,no_subtree_check,root_squash)
[root@opennebula01 ~]#

So I dug deeper and see that there is a possibility libvirtd is trying
to access that NFS mount as root as some level because as root I also
get a permission denied to the NFS share above. Rightly so since I have
root_squash that I need to keep. But libvirtd should be able to access
the file as oneadmin as I have above. It's not and this is what I read
on it:

https://www.redhat.com/archives/libvir-list/2014-May/msg00194.html

Comment is: "The current implementation works for local
storage only and returns the canonical path of the volume."

But it seems the logic is applied to NFS mounts. Perhaps it shouldn't
be? Anyway to get around this problem? This is CentOS 7 .

My post with OpenNebula is here from which this conversation originates:
https://forum.opennebula.org/t/libvirtd-running-as-root-tries-to-access-oneadmin-nfs-mount-error-cant-canonicalize-path/2054/7

Cheers,
Tom K.
-

Living on earth is expensive, but it includes a free trip around the sun.

On 4/12/2016 10:03 AM, Martin Kletzander wrote:

On Mon, Apr 11, 2016 at 08:02:04PM -0400, TomK wrote:

Hey All,

Wondering if anyone had any suggestions on this topic?

The only thing I can come up with is:
'/var/lib/one//datastores/0/38/disk.1': Permission denied

... that don't have access to that file. Could you elaborate on that?

I think it's either:

a) you are running the domain as root or

b) we don't use the domain's uid/gid to canonicalize the path.

But if read access is enough for canonicalizing that path, I think the
problem is purely with permissions.

Cheers,
Tom K.
-

Living on earth is expensive, but it includes a free trip around the
sun.

On 4/9/2016 11:08 AM, TomK wrote:

Adding in libvir-list.

Cheers,
Tom K.
-

Living on earth is expensive, but it includes a free trip around the
sun.

On 4/7/2016 7:32 PM, TomK wrote:

Hey All,

I've an issue where libvirtd tries to access an NFS mount but errors
out with: can't canonicalize path '/var/lib/one//datastores/0 . The
unprevilidged user is able to read/write fine to the share.
root_squash is used and for security reasons no_root_squash cannot be
used.

On the controller and node SELinux is disabled.

[oneadmin@mdskvm-p01 ~]$ virsh -d 1 --connect qemu:///system create
/var/lib/one//datastores/0/38/deployment.0
create: file(optdata): /var/lib/one//datastores/0/38/deployment.0
error: Failed to create domain from
/var/lib/one//datastores/0/38/deployment.0
error: can't canonicalize path
'/var/lib/one//datastores/0/38/disk.1': Permission denied

I added some debug flags to get more info and added -x to the deploy
script. Closest I get to more details is this:

2016-04-06 04:15:35.945+: 14072: debug :
virStorageFileBackendFileInit:1441 : initializing FS storage file
0x7f6aa4009000 (file:/var/lib/one//datastores/0/38/disk.1)[9869:9869]
2016-04-06 04:15:35.954+: 14072: error :

Re: [libvirt] [PATCH v2 10/10] vz: change vzConnectIsAlive behavior



On 07.04.2016 23:10, Maxim Nestratov wrote:
> Now it detects if a connection to vz dispatcher is up or not by
> comparing current driver to connection stored pointer.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_driver.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index b3ce404..7d7cb17 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -824,8 +824,12 @@ static int vzConnectIsSecure(virConnectPtr conn 
> ATTRIBUTE_UNUSED)
>  return 1;
>  }
>  
> -static int vzConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED)
> +static int vzConnectIsAlive(virConnectPtr conn)
>  {
> +vzConnPtr privconn = conn->privateData;
> +if (privconn->driver != vz_driver)
> +return 0;
> +
>  return 1;
>  }
>  
> 

Looks like we never get here because now we live only in daemon
and remote driver will never ask us for this function. Remote driver
only checks connection to daemon.

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

[libvirt] [PATCH 06/12] qemu: hotplug: Assume support for -device for attach virtio disk

Since support for QEMU_CAPS_DEVICE is not assumed, let's drop the legacy
code to make life easier going forward.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_hotplug.c | 79 +
 1 file changed, 33 insertions(+), 46 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 05fa787..f308738 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -316,7 +316,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
  virDomainDiskDefPtr disk)
 {
 int ret = -1;
-const char* type = virDomainDiskBusTypeToString(disk->bus);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 char *devstr = NULL;
 char *drivestr = NULL;
@@ -340,62 +339,50 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
 goto cleanup;
 
-if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
-if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
-if (virDomainCCWAddressAssign(>info, priv->ccwaddrs,
-  !disk->info.addr.ccw.assigned) < 0)
-goto error;
-} else if (!disk->info.type ||
-disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0)
-goto error;
-}
-releaseaddr = true;
-if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
+if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+if (virDomainCCWAddressAssign(>info, priv->ccwaddrs,
+  !disk->info.addr.ccw.assigned) < 0)
 goto error;
-
-if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+} else if (!disk->info.type ||
+disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0)
 goto error;
+}
+releaseaddr = true;
+if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
+goto error;
 
-if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
-goto error;
+if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+goto error;
 
-if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
-goto error;
+if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+goto error;
 
-if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
-goto error;
-}
+if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
+goto error;
+
+if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
+goto error;
 
 if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
 goto error;
 
 qemuDomainObjEnterMonitor(driver, vm);
-if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
-ret = qemuMonitorAddDrive(priv->mon, drivestr);
-if (ret == 0) {
-ret = qemuMonitorAddDevice(priv->mon, devstr);
-if (ret < 0) {
-virErrorPtr orig_err = virSaveLastError();
-if (!drivealias ||
-qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
-VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- NULLSTR(drivealias), drivestr);
-}
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+ret = qemuMonitorAddDrive(priv->mon, drivestr);
+if (ret == 0) {
+ret = qemuMonitorAddDevice(priv->mon, devstr);
+if (ret < 0) {
+virErrorPtr orig_err = virSaveLastError();
+if (!drivealias ||
+qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ NULLSTR(drivealias), drivestr);
+}
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
 }
-}
-} else if (!disk->info.type ||
-disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-virDevicePCIAddress guestAddr = disk->info.addr.pci;
-ret = qemuMonitorAddPCIDisk(priv->mon, src, type, );
-if (ret == 0) {
-disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-memcpy(>info.addr.pci, , sizeof(guestAddr));
 }
 }
 if (qemuDomainObjExitMonitor(driver, vm) < 0) {
-- 
2.5.5

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

Re: [libvirt] [PATCH 08/15] conf: utility function to convert PCI controller model into connect type

2016-04-12 Thread Laine Stump


On 04/05/2016 08:33 AM, Ján Tomko wrote:

On Thu, Mar 24, 2016 at 03:25:43PM -0400, Laine Stump wrote:

There are two places in qemu_domain_address.c where we have a switch
statement to convert PCI controller models
(VIR_DOMAIN_CONTROLLER_MODEL_PCI*) into the connection type flag that
is matched when looking for an upstream connection for that model of
controller (VIR_PCI_CONNECT_TYPE_*). This patch makes a utility
function in conf/domain_addr.c to do that, so that when a new PCI
controller is added, we only need to add the new model-->connect-type
in a single place.
---
  src/conf/domain_addr.c | 47 +
  src/conf/domain_addr.h |  4 +++
  src/libvirt_private.syms   |  1 +
  src/qemu/qemu_domain_address.c | 80 +-
  4 files changed, 61 insertions(+), 71 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 4408c4a..1bf2c9a 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -32,6 +32,53 @@
  
  VIR_LOG_INIT("conf.domain_addr");
  
+int

+virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model,
+ virDomainPCIConnectFlags *connectType)
+{
+/* given a VIR_DOMAIN_CONTROLLER_MODEL_PCI*, set connectType to
+ * the equivalent VIR_PCI_CONNECT_TYPE_*. return 0 on success, -1
+ * if the model wasn't recognized.
+ */
+switch (model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+/* pci-root and pcie-root are implicit in the machine,
+ * and have no upstream connection
+ */
+*connectType = 0;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+/* pci-bridge is treated like a standard PCI endpoint device, */
+*connectType = VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+/* dmi-to-pci-bridge is treated like a PCIe device
+ * (e.g. it can be plugged directly into pcie-root)
+ */
+*connectType = VIR_PCI_CONNECT_TYPE_PCIE_DEVICE;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+*connectType = VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+*connectType = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+*connectType = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT;
+break;
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+/* if this happens, there is an error in the code. A
+ * PCI controller should always have a proper model
+ * set
+ */
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("PCI controller model incorrectly set to 'last'"));
+return -1;

This is dead code.
The function can just return virDomainPCIConnectFlags directly.


I was originally going to do that, but since the switch is over an enum, 
all possible values must have cases. I suppose I could just hand-wave 
over that case saying "This will never happen" and have it return 0. 
Actually, I think I *will* do that; it would be reasonable to think that 
someone would screw up by failing to set the model (in which case it 
would be 0, i.e. "...PCI_ROOT"), but it would take some serious ill 
intent to set it to "...PCI_LAST"


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

[libvirt] [PATCH 09/12] qemu: hotplug: Adjust error path for attach hostdev scsi disk

Shortly a new object could be added making this code even more confusing,
so let's just adjust the exit path now to make it clearer.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_hotplug.c | 56 ++---
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ec30ce7..b6cf196 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -602,7 +602,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
 goto failadddevice;
 
 if (qemuDomainObjExitMonitor(driver, vm) < 0)
-goto failexitmon;
+goto failexitmonitor;
 
 virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
 
@@ -623,7 +623,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
  failadddrive:
 ignore_value(qemuDomainObjExitMonitor(driver, vm));
 
- failexitmon:
+ failexitmonitor:
 virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
 
  error:
@@ -1929,6 +1929,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
 {
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
+virErrorPtr orig_err;
 virDomainControllerDefPtr cont = NULL;
 char *devstr = NULL;
 char *drvstr = NULL;
@@ -1989,32 +1990,24 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
 if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
 goto cleanup;
 
+/* Attach the device - 2 step process */
 qemuDomainObjEnterMonitor(driver, vm);
-if ((ret = qemuMonitorAddDrive(priv->mon, drvstr)) == 0) {
-if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0) {
-virErrorPtr orig_err = virSaveLastError();
-if (qemuMonitorDriveDel(priv->mon, drvstr) < 0)
-VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- drvstr, devstr);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
-}
-}
-if (qemuDomainObjExitMonitor(driver, vm) < 0) {
-ret = -1;
-goto cleanup;
-}
 
-virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
-if (ret < 0)
-goto cleanup;
+if (qemuMonitorAddDrive(priv->mon, drvstr) < 0)
+goto failadddrive;
+
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+goto failadddevice;
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto failexitmonitor;
+
+virDomainAuditHostdev(vm, hostdev, "attach", true);
 
 vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
 
 ret = 0;
+
  cleanup:
 qemuDomainSecretHostdevDestroy(hostdev);
 if (ret < 0) {
@@ -2029,6 +2022,25 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
 VIR_FREE(drvstr);
 VIR_FREE(devstr);
 return ret;
+
+ failadddevice:
+orig_err = virSaveLastError();
+if (qemuMonitorDriveDel(priv->mon, drvstr) < 0)
+VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ drvstr, devstr);
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
+}
+
+ failadddrive:
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ failexitmonitor:
+virDomainAuditHostdev(vm, hostdev, "attach", false);
+
+goto cleanup;
 }
 
 
-- 
2.5.5

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

[libvirt] [PATCH 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy

Rather than needing to pass the conn parameter to various command
line building API's, add qemuDomainSecretPrepare just prior to the
qemuProcessLaunch which calls qemuBuilCommandLine. The function
must be called after qemuProcessPrepareHost since it's expected
to eventually need the domain masterKey generated during the prepare
host call. Additionally, future patches may require device aliases
(assigned during the prepare domain call) in order to associate
the secret objects.

The qemuDomainSecretDestroy is called after the qemuProcessLaunch
finishes in order to clear and free memory used by the secrets
that were recently prepared, so they are not kept around in memory
too long.

Placing the setup here is beneficial for future patches which will
need the domain masterKey in order to generate an encrypted secret
along with an initialization vector to be saved and passed (since
the masterKey shouldn't be passed around).

Finally, since the secret is not added during command line build,
the hotplug code will need to get the secret into the private disk data.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_command.c |  45 ---
 src/qemu/qemu_command.h |   5 +-
 src/qemu/qemu_domain.c  | 150 ++--
 src/qemu/qemu_domain.h  |  15 -
 src/qemu/qemu_driver.c  |  10 ++--
 src/qemu/qemu_hotplug.c |  26 +
 src/qemu/qemu_hotplug.h |   1 -
 src/qemu/qemu_process.c |   8 +++
 8 files changed, 202 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bd82682..a804987 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -832,7 +832,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 
 int
 qemuGetDriveSourceString(virStorageSourcePtr src,
- virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
  char **source)
 {
 int actualType = virStorageSourceGetActualType(src);
@@ -846,31 +846,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
 if (virStorageSourceIsEmpty(src))
 return 1;
 
-if (conn) {
-if (actualType == VIR_STORAGE_TYPE_NETWORK &&
-src->auth &&
-(src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
- src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
-bool encode = false;
-int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
-const char *protocol = 
virStorageNetProtocolTypeToString(src->protocol);
-username = src->auth->username;
-
-if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
-/* qemu requires the secret to be encoded for RBD */
-encode = true;
-secretType = VIR_SECRET_USAGE_TYPE_CEPH;
-}
-
-if (!(secret = virSecretGetSecretString(conn,
-protocol,
-encode,
-src->auth,
-secretType)))
-goto cleanup;
-}
-}
-
 switch ((virStorageType) actualType) {
 case VIR_STORAGE_TYPE_BLOCK:
 case VIR_STORAGE_TYPE_FILE:
@@ -881,6 +856,11 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
 break;
 
 case VIR_STORAGE_TYPE_NETWORK:
+if (secinfo) {
+username = secinfo->s.plain.username;
+secret = secinfo->s.plain.secret;
+}
+
 if (!(*source = qemuBuildNetworkDriveURI(src, username, secret)))
 goto cleanup;
 break;
@@ -894,7 +874,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
 ret = 0;
 
  cleanup:
-VIR_FREE(secret);
 return ret;
 }
 
@@ -1033,8 +1012,7 @@ qemuCheckFips(void)
 
 
 char *
-qemuBuildDriveStr(virConnectPtr conn,
-  virDomainDiskDefPtr disk,
+qemuBuildDriveStr(virDomainDiskDefPtr disk,
   bool bootable,
   virQEMUCapsPtr qemuCaps)
 {
@@ -1046,6 +1024,7 @@ qemuBuildDriveStr(virConnectPtr conn,
 int busid = -1, unitid = -1;
 char *source = NULL;
 int actualType = virStorageSourceGetActualType(disk->src);
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
 
 if (idx < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1127,7 +1106,7 @@ qemuBuildDriveStr(virConnectPtr conn,
 break;
 }
 
-if (qemuGetDriveSourceString(disk->src, conn, ) < 0)
+if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, ) < 0)
 goto error;
 
 if (source &&
@@ -1816,7 +1795,6 @@ qemuBuildDriveDevStr(const virDomainDef *def,
 
 static int
 qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
-  virConnectPtr conn,
   const virDomainDef *def,
   virQEMUCapsPtr qemuCaps,

[libvirt] [PATCH 10/12] qemu: hotplug: Fix possible memory leak of props

If we failed to build the aliases or attach the chardev, then the props
would be leaked - fix that.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_hotplug.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b6cf196..c82d455 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1685,6 +1685,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
 
 if (qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0)
 goto failbackend;
+props = NULL;
 
 if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
 goto failfrontend;
@@ -1702,6 +1703,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
  audit:
 virDomainAuditRNG(vm, NULL, rng, "attach", ret == 0);
  cleanup:
+virJSONValueFree(props);
 if (ret < 0 && vm)
 qemuDomainReleaseDeviceAddress(vm, >info, NULL);
 VIR_FREE(charAlias);
@@ -1715,6 +1717,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
  failbackend:
 if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
 ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
+props = NULL;  /* qemuMonitorAddObject consumes on failure */
  failchardev:
 if (qemuDomainObjExitMonitor(driver, vm) < 0) {
 vm = NULL;
-- 
2.5.5

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

[libvirt] [PATCH 08/12] qemu: hotplug: Adjust error path for attach scsi disk

Shortly a new object could be added making this code even more confusing,
so let's just adjust the exit path now to make it clearer.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_hotplug.c | 35 +++
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c3edd40..ec30ce7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -592,29 +592,22 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
 if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
 goto error;
 
+/* Attach the device - 2 step process */
 qemuDomainObjEnterMonitor(driver, vm);
 
-ret = qemuMonitorAddDrive(priv->mon, drivestr);
-if (ret == 0) {
-ret = qemuMonitorAddDevice(priv->mon, devstr);
-if (ret < 0) {
-VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
- drivestr, devstr);
-/* XXX should call 'drive_del' on error but this does not exist 
yet */
-}
-}
+if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+goto failadddrive;
 
-if (qemuDomainObjExitMonitor(driver, vm) < 0) {
-ret = -1;
-goto error;
-}
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+goto failadddevice;
 
-virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto failexitmon;
 
-if (ret < 0)
-goto error;
+virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
 
 virDomainDiskInsertPreAlloced(vm->def, disk);
+ret = 0;
 
  cleanup:
 qemuDomainSecretDiskDestroy(disk);
@@ -623,6 +616,16 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
 virObjectUnref(cfg);
 return ret;
 
+ failadddevice:
+/* XXX should call 'drive_del' on error but this does not exist yet */
+VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr);
+
+ failadddrive:
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ failexitmon:
+virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
+
  error:
 ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
 goto cleanup;
-- 
2.5.5

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

[libvirt] [PATCH 04/12] qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy

Similar to the qemuDomainSecretDiskPrepare, generate the secret
for the Hostdev's prior to call qemuProcessLaunch which calls
qemuBuildCommandLine. Additionally, since the secret is not longer
added as part of building the command, the hotplug code will need
to make the call to add the secret in the hostdevPriv.

Since this then is the last requirement to pass a virConnectPtr
to qemuBuildCommandLine, we now can remove that as part of these
changes. That removal has cascading effects through various callers.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_command.c | 42 ++--
 src/qemu/qemu_command.h | 11 +++-
 src/qemu/qemu_domain.c  | 73 +
 src/qemu/qemu_domain.h  |  7 +
 src/qemu/qemu_driver.c  |  3 +-
 src/qemu/qemu_hotplug.c | 31 +
 src/qemu/qemu_hotplug.h |  3 +-
 src/qemu/qemu_process.c |  5 ++--
 8 files changed, 122 insertions(+), 53 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a804987..0726abf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -50,7 +50,6 @@
 #include "secret_conf.h"
 #include "network/bridge_driver.h"
 #include "virnetdevtap.h"
-#include "secret_util.h"
 #include "device_conf.h"
 #include "virstoragefile.h"
 #include "virtpm.h"
@@ -4412,29 +4411,24 @@ qemuBuildSCSIHostHostdevDrvStr(virDomainHostdevDefPtr 
dev,
 }
 
 static char *
-qemuBuildSCSIiSCSIHostdevDrvStr(virConnectPtr conn,
-virDomainHostdevDefPtr dev)
+qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
 {
 char *source = NULL;
 char *secret = NULL;
 char *username = NULL;
 virStorageSource src;
+qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev);
 
 memset(, 0, sizeof(src));
 
 virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
 virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = >u.iscsi;
 
-if (conn && iscsisrc->auth) {
-const char *protocol =
-virStorageNetProtocolTypeToString(VIR_STORAGE_NET_PROTOCOL_ISCSI);
-bool encode = false;
-int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
+if (hostdevPriv->secinfo) {
+qemuDomainSecretInfoPtr secinfo = hostdevPriv->secinfo;
 
-username = iscsisrc->auth->username;
-if (!(secret = virSecretGetSecretString(conn, protocol, encode,
-iscsisrc->auth, secretType)))
-goto cleanup;
+username = secinfo->s.plain.username;
+secret = secinfo->s.plain.secret;
 }
 
 src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
@@ -4445,14 +4439,11 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virConnectPtr conn,
 /* Rather than pull what we think we want - use the network disk code */
 source = qemuBuildNetworkDriveURI(, username, secret);
 
- cleanup:
-VIR_FREE(secret);
 return source;
 }
 
 char *
-qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
-   virDomainHostdevDefPtr dev,
+qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps,
qemuBuildCommandLineCallbacksPtr callbacks)
 {
@@ -4461,7 +4452,7 @@ qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
 virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
 
 if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
-if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(conn, dev)))
+if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(dev)))
 goto error;
 virBufferAsprintf(, "file=%s,if=none,format=raw", source);
 } else {
@@ -4760,7 +4751,6 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 
 static int
 qemuBuildHostdevCommandLine(virCommandPtr cmd,
-virConnectPtr conn,
 const virDomainDef *def,
 virQEMUCapsPtr qemuCaps,
 qemuBuildCommandLineCallbacksPtr callbacks,
@@ -4910,8 +4900,8 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
 char *drvstr;
 
 virCommandAddArg(cmd, "-drive");
-if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev,
-  qemuCaps, 
callbacks)))
+if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps,
+  callbacks)))
 return -1;
 virCommandAddArg(cmd, drvstr);
 VIR_FREE(drvstr);
@@ -9148,13 +9138,9 @@ qemuBuildCommandLineCallbacks buildCommandLineCallbacks 
= {
 /*
  * Constructs a argv suitable for launching qemu with config defined
  * for a given virtual machine.
- *
- * XXX 'conn' is only required to resolve network -> bridge name
- * figure out how to remove this requirement some day
  */
 virCommandPtr

[libvirt] [PATCH 11/12] qemu: Introduce qemuDomainSecretIV

Add the data structure and infrastructure to support an initialization
vector (IV) secret.  The IV secret generation will need to have access
to the domain private master key, so let's make sure the prepare disk
and hostdev functions can accept that now.

Anywhere that needs to make a decision over which secret type to use
in order to fill in or use the IV secret has a switch added. In particular,
when building the command line for the network URI, create a couple of
helper functions which make the decision to add the secret info.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_command.c | 106 ++--
 src/qemu/qemu_domain.c  |  34 ++--
 src/qemu/qemu_domain.h  |  22 --
 src/qemu/qemu_hotplug.c |   6 +--
 4 files changed, 138 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 31e02de..3a69bd5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -606,6 +606,85 @@ qemuNetworkDriveGetPort(int protocol,
 return -1;
 }
 
+
+/* qemuBuildGeneralSecinfoURI:
+ * @uri: Pointer to the URI structure to add to
+ * @secinfo: Pointer to the secret info data (if present)
+ *
+ * If we have a secinfo, then build the command line options for
+ * the secret info for the "general" case (somewhat a misnomer since
+ * an iscsi disk is the only one with a secinfo).
+ *
+ * Returns 0 on success or if no secinfo,
+ * -1 and error message if fail to add secret information
+ */
+static int
+qemuBuildGeneralSecinfoURI(virURIPtr uri,
+   qemuDomainSecretInfoPtr secinfo)
+{
+if (!secinfo)
+return 0;
+
+switch ((qemuDomainSecretInfoType) secinfo->type) {
+case VIR_DOMAIN_SECRET_INFO_PLAIN:
+if (secinfo->s.plain.secret) {
+if (virAsprintf(>user, "%s:%s",
+secinfo->s.plain.username,
+secinfo->s.plain.secret) < 0)
+return -1;
+} else {
+if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
+return -1;
+}
+break;
+
+case VIR_DOMAIN_SECRET_INFO_IV:
+case VIR_DOMAIN_SECRET_INFO_LAST:
+return -1;
+}
+
+return 0;
+}
+
+
+/* qemuBuildRBDSecinfoURI:
+ * @uri: Pointer to the URI structure to add to
+ * @secinfo: Pointer to the secret info data (if present)
+ *
+ * If we have a secinfo, then build the command line options for
+ * the secret info for the RBD network storage. Assumption for this
+ * is both username and secret exist for plaintext
+ *
+ * Returns 0 on success or if no secinfo,
+ * -1 and error message if fail to add secret information
+ */
+static int
+qemuBuildRBDSecinfoURI(virBufferPtr buf,
+   qemuDomainSecretInfoPtr secinfo)
+{
+if (!secinfo) {
+virBufferAddLit(buf, ":auth_supported=none");
+return 0;
+}
+
+switch ((qemuDomainSecretInfoType) secinfo->type) {
+case VIR_DOMAIN_SECRET_INFO_PLAIN:
+virBufferEscape(buf, '\\', ":", ":id=%s",
+secinfo->s.plain.username);
+virBufferEscape(buf, '\\', ":",
+":key=%s:auth_supported=cephx\\;none",
+secinfo->s.plain.secret);
+break;
+
+case VIR_DOMAIN_SECRET_INFO_IV:
+case VIR_DOMAIN_SECRET_INFO_LAST:
+return -1;
+}
+
+return 0;
+}
+
+
 #define QEMU_DEFAULT_NBD_PORT "10809"
 
 static char *
@@ -701,7 +780,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 goto cleanup;
 }
 
-if ((uri->port = qemuNetworkDriveGetPort(src->protocol, 
src->hosts->port)) < 0)
+if ((uri->port = qemuNetworkDriveGetPort(src->protocol,
+ src->hosts->port)) < 0)
 goto cleanup;
 
 if (src->path) {
@@ -721,17 +801,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 virAsprintf(>query, "socket=%s", src->hosts->socket) < 0)
 goto cleanup;
 
-if (secinfo) {
-if (secinfo->s.plain.secret) {
-if (virAsprintf(>user, "%s:%s",
-secinfo->s.plain.username,
-secinfo->s.plain.secret) < 0)
-goto cleanup;
-} else {
-if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
-goto cleanup;
-}
-}
+if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0)
+goto cleanup;
 
 if (VIR_STRDUP(uri->server, src->hosts->name) < 0)
 goto cleanup;
@@ -777,15 +848,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 if (src->snapshot)
 virBufferEscape(, '\\', ":", "@%s", src->snapshot);
 
-if (secinfo) {
-

[libvirt] [PATCH 07/12] qemu: hotplug: Adjust error path for attach virtio disk

Shortly a new object could be added making this code even more confusing,
so let's just adjust the exit path now to make it clearer.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_hotplug.c | 55 -
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f308738..c3edd40 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -317,6 +317,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 {
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
+virErrorPtr orig_err;
 char *devstr = NULL;
 char *drivestr = NULL;
 char *drivealias = NULL;
@@ -367,36 +368,24 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
 goto error;
 
+/* Attach the device - 2 step process */
 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorAddDrive(priv->mon, drivestr);
-if (ret == 0) {
-ret = qemuMonitorAddDevice(priv->mon, devstr);
-if (ret < 0) {
-virErrorPtr orig_err = virSaveLastError();
-if (!drivealias ||
-qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
-VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- NULLSTR(drivealias), drivestr);
-}
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
-}
-}
+
+if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+goto failadddrive;
+
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+goto failadddevice;
+
 if (qemuDomainObjExitMonitor(driver, vm) < 0) {
 releaseaddr = false;
-ret = -1;
-goto error;
+goto failexitmonitor;
 }
 
-virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
-
-if (ret < 0)
-goto error;
+virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
 
 virDomainDiskInsertPreAlloced(vm->def, disk);
+ret = 0;
 
  cleanup:
 qemuDomainSecretDiskDestroy(disk);
@@ -406,6 +395,26 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
 virObjectUnref(cfg);
 return ret;
 
+ failadddevice:
+orig_err = virSaveLastError();
+if (!drivealias ||
+qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ NULLSTR(drivealias), drivestr);
+}
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
+}
+
+ failadddrive:
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+releaseaddr = false;
+
+ failexitmonitor:
+virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
+
  error:
 if (releaseaddr)
 qemuDomainReleaseDeviceAddress(vm, >info, src);
-- 
2.5.5

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

[libvirt] [PATCH 05/12] qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI

Rather than take username and password as parameters, now take
a qemuDomainSecretInfoPtr and decode within the function.

NB: Having secinfo implies having the username for a plain type
from a successful virSecretGetSecretString

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_command.c | 40 +---
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0726abf..31e02de 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -610,8 +610,7 @@ qemuNetworkDriveGetPort(int protocol,
 
 static char *
 qemuBuildNetworkDriveURI(virStorageSourcePtr src,
- const char *username,
- const char *secret)
+ qemuDomainSecretInfoPtr secinfo)
 {
 char *ret = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -722,12 +721,14 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 virAsprintf(>query, "socket=%s", src->hosts->socket) < 0)
 goto cleanup;
 
-if (username) {
-if (secret) {
-if (virAsprintf(>user, "%s:%s", username, secret) < 0)
+if (secinfo) {
+if (secinfo->s.plain.secret) {
+if (virAsprintf(>user, "%s:%s",
+secinfo->s.plain.username,
+secinfo->s.plain.secret) < 0)
 goto cleanup;
 } else {
-if (VIR_STRDUP(uri->user, username) < 0)
+if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
 goto cleanup;
 }
 }
@@ -776,11 +777,12 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
 if (src->snapshot)
 virBufferEscape(, '\\', ":", "@%s", src->snapshot);
 
-if (username) {
-virBufferEscape(, '\\', ":", ":id=%s", username);
+if (secinfo) {
+virBufferEscape(, '\\', ":", ":id=%s",
+secinfo->s.plain.username);
 virBufferEscape(, '\\', ":",
 ":key=%s:auth_supported=cephx\\;none",
-secret);
+secinfo->s.plain.secret);
 } else {
 virBufferAddLit(, ":auth_supported=none");
 }
@@ -835,8 +837,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
  char **source)
 {
 int actualType = virStorageSourceGetActualType(src);
-char *secret = NULL;
-char *username = NULL;
 int ret = -1;
 
 *source = NULL;
@@ -855,12 +855,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
 break;
 
 case VIR_STORAGE_TYPE_NETWORK:
-if (secinfo) {
-username = secinfo->s.plain.username;
-secret = secinfo->s.plain.secret;
-}
-
-if (!(*source = qemuBuildNetworkDriveURI(src, username, secret)))
+if (!(*source = qemuBuildNetworkDriveURI(src, secinfo)))
 goto cleanup;
 break;
 
@@ -4414,8 +4409,6 @@ static char *
 qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
 {
 char *source = NULL;
-char *secret = NULL;
-char *username = NULL;
 virStorageSource src;
 qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev);
 
@@ -4424,20 +4417,13 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr 
dev)
 virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
 virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = >u.iscsi;
 
-if (hostdevPriv->secinfo) {
-qemuDomainSecretInfoPtr secinfo = hostdevPriv->secinfo;
-
-username = secinfo->s.plain.username;
-secret = secinfo->s.plain.secret;
-}
-
 src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
 src.path = iscsisrc->path;
 src.hosts = iscsisrc->hosts;
 src.nhosts = iscsisrc->nhosts;
 
 /* Rather than pull what we think we want - use the network disk code */
-source = qemuBuildNetworkDriveURI(, username, secret);
+source = qemuBuildNetworkDriveURI(, hostdevPriv->secinfo);
 
 return source;
 }
-- 
2.5.5

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

[libvirt] [PATCH 12/12] qemu: Utilize qemu secret objects for SCSI/RBD auth/secret

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

If they're available and we need to pass secrets to qemu, then use the
qemu domain secret object in order to pass the secrets for iSCSI and
RBD volumes instead of passing plaintext or base64 encoded secrets on
the command line.

New APIs:

  qemuDomainGetIVKeyAlias:
Generate/return the secret object alias for an initialization
vector (IV) secret info type. This will be saved in the secret
info block.

  qemuDomainSecretInfoGetAlias:
Return a pointer to the alias to the specific secret info as
long as the secret object is supported (future patches may add
a new secret info type with a different pointer to return).

  qemuDomainSecretHaveEncrypt:
Boolean function to determine whether the underly encryption
API is available. This function will utilize a similar mechanism
as the 'gnutls_rnd' did in configure.ac. This function creates
the encrypted secret based upon the domain master key, an
initialization vector (16 byte random value), and the stored secret.

  qemuDomainSecretIVSetup: (private)
This API handles the details of the generation of the IV secret
and saves the pieces that need to be passed to qemu in order for
the secret to be decrypted. The requirement from qemu is the IV
and encrypted secret are to be base64 encoded. They can be passed
either directly or within a file. This implementation chooses
to pass directly rather than a file (file passing is shown below).

  qemuBuildSecretInfoProps:
Generate/return a JSON properties object for the IV secret to
be used by both the command building and hotplug code in order
to add the secret object.

Changes for qemuDomainSecret{Disk|Hostdev}Prepare:

  If both the encryption API exists and the secret object exist, then
  setup the IV secret (qemuDomainSecretIVSetup) as the default means
  for the disk/hostdev to provide the secret to qemu. Prior to command
  line generation and during hotplug, these prepare API's are called
  allowing for code after the API to perform the right steps.

Command Building:

  Adjust the qemuBuild{General|RBD}SecinfoURI API's in order to generate
  the specific command options for an IV secret, such as:

  For iSCSI:

-object secret,id=sec0,keyid=$masterKey,filename=/path/to/example.pw

   or

-object secret,id=sec0,keyid=$masterKey,data=$base64encodedencrypted,
format=base64

-drive driver=iscsi,url=iscsi://example.com/target-foo/lun1,
   user=dan,password-secret=sec0

  For RBD:

-object secret,id=secret0,keyid=$masterKey,file=/path/to/poolkey.b64,
format=base64

   or

-object secret,id=secret0,keyid=$masterKey,data=$base64encodedencrypted,
format=base64

-drive driver=rbd,filename=rbd:pool/image:id=myname:
   auth_supported=cephx,password-secret=secret0

  where for both 'id=' value is the secret object alias, the 'keyid=
  $masterKey' is the master key shared with qemu, and the -drive
  syntax will reference that alias as the 'password-secret'. For
  the iSCSI object 'user=' replaces the URI generated 'user:secret@'
  prepended to the iSCSI 'host' name (example.com). For the RBD -drive
  syntax, the 'id=myname' is kept to define the username, while the
  'key=$base64 encoded secret' is removed.

  While according to the syntax described for qemu commits 'b189346eb'
  (iSCSI) and '60390a21' (RBD) or as seen in the email archive:

https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04083.html

  it is possible to pass a plaintext password via a file, the qemu
  commit 'ac1d8878' describes the more feature rich 'keyid=' option
  based upon the shared masterKey.

Hotplug Adjustments:
  Once the qemuDomainSecret{Disk|Hostdev}Prepare completes successfully,
  a check (qemuDomainSecretInfoGetAlias) to determine whether the IV
  secret alias is available results in generating the JSON props (or not).
  (via qemuBuildSecretInfoProps). If the secret alias object exists, then
  prior to adding the device and drive, the secret object will be added
  to define the 'password-secret' parameter.

The goal is to make this the default and have no user interaction
required in order to allow using the IV mechanism. If the mechanism
is not available, then fall back to the current mechanism.

Signed-off-by: John Ferlan 
---
 configure.ac|   1 +
 src/qemu/qemu_alias.c   |  23 ++
 src/qemu/qemu_alias.h   |   2 +
 src/qemu/qemu_command.c | 126 ++
 src/qemu/qemu_command.h |   4 +
 src/qemu/qemu_domain.c  | 199 +---
 src/qemu/qemu_domain.h  |   3 +
 src/qemu/qemu_hotplug.c |  72 +-
 8 files changed, 418 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1eb19ee..83a1043 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1295,6 +1295,7 @@ if test "x$with_gnutls" != "xno"; then

[libvirt] [PATCH 03/12] qemu: Introduce qemuDomainHostdevPrivatePtr

Modeled after the qemuDomainDiskPrivatePtr logic, create a privateData
pointer in the _virDomainHostdevDef to allow storage of private data
for a hypervisor in order to at least temporarily store auth/secrets
data for usage during qemuBuildCommandLine.

NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the auth/secret data, there's no need to add
code to handle this new structure there.

Updated copyrights for modules touched. Some didn't have updates in a
couple years even though changes have been made.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c| 28 +--
 src/conf/domain_conf.h|  7 +--
 src/lxc/lxc_native.c  |  4 ++--
 src/qemu/qemu_domain.c| 44 +++
 src/qemu/qemu_domain.h| 13 +
 src/qemu/qemu_parse_command.c |  4 ++--
 src/vbox/vbox_common.c|  4 ++--
 src/xenconfig/xen_common.c|  4 ++--
 src/xenconfig/xen_sxpr.c  |  4 ++--
 tests/virhostdevtest.c|  3 ++-
 10 files changed, 96 insertions(+), 19 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 48c7bc5..55b16dc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2115,16 +2115,29 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
 VIR_FREE(def);
 }
 
-virDomainHostdevDefPtr virDomainHostdevDefAlloc(void)
+
+virDomainHostdevDefPtr
+virDomainHostdevDefAlloc(virDomainXMLOptionPtr xmlopt)
 {
 virDomainHostdevDefPtr def = NULL;
 
 if (VIR_ALLOC(def) < 0 ||
 VIR_ALLOC(def->info) < 0)
 VIR_FREE(def);
+
+if (xmlopt &&
+xmlopt->privateData.hostdevNew &&
+!(def->privateData = xmlopt->privateData.hostdevNew()))
+goto error;
+
 return def;
+
+ error:
+virDomainHostdevDefFree(def);
+return NULL;
 }
 
+
 static void
 virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr 
iscsisrc)
 {
@@ -12271,7 +12284,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
 }
 
 static virDomainHostdevDefPtr
-virDomainHostdevDefParseXML(xmlNodePtr node,
+virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
+xmlNodePtr node,
 xmlXPathContextPtr ctxt,
 virHashTablePtr bootHash,
 unsigned int flags)
@@ -12283,7 +12297,7 @@ virDomainHostdevDefParseXML(xmlNodePtr node,
 
 ctxt->node = node;
 
-if (!(def = virDomainHostdevDefAlloc()))
+if (!(def = virDomainHostdevDefAlloc(xmlopt)))
 goto error;
 
 if (mode) {
@@ -12933,8 +12947,9 @@ virDomainDeviceDefParse(const char *xmlStr,
 goto error;
 break;
 case VIR_DOMAIN_DEVICE_HOSTDEV:
-if (!(dev->data.hostdev = virDomainHostdevDefParseXML(node, ctxt,
-  NULL, flags)))
+if (!(dev->data.hostdev = virDomainHostdevDefParseXML(xmlopt, node,
+  ctxt, NULL,
+  flags)))
 goto error;
 break;
 case VIR_DOMAIN_DEVICE_CONTROLLER:
@@ -16454,7 +16469,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 for (i = 0; i < n; i++) {
 virDomainHostdevDefPtr hostdev;
 
-hostdev = virDomainHostdevDefParseXML(nodes[i], ctxt, bootHash, flags);
+hostdev = virDomainHostdevDefParseXML(xmlopt, nodes[i], ctxt,
+  bootHash, flags);
 if (!hostdev)
 goto error;
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6f93def..d442866 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1,7 +1,7 @@
 /*
  * domain_conf.h: domain XML processing
  *
- * Copyright (C) 2006-2015 Red Hat, Inc.
+ * Copyright (C) 2006-2016 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
  *
@@ -543,6 +543,8 @@ struct _virDomainHostdevCaps {
 /* basic device for direct passthrough */
 struct _virDomainHostdevDef {
 virDomainDeviceDef parent; /* higher level Def containing this */
+virObjectPtr privateData;
+
 int mode; /* enum virDomainHostdevMode */
 int startupPolicy; /* enum virDomainStartupPolicy */
 bool managed;
@@ -2486,6 +2488,7 @@ struct _virDomainXMLPrivateDataCallbacks {
 virDomainXMLPrivateDataAllocFunc  alloc;
 virDomainXMLPrivateDataFreeFunc   free;
 virDomainXMLPrivateDataNewFuncdiskNew;
+virDomainXMLPrivateDataNewFunchostdevNew;
 virDomainXMLPrivateDataFormatFunc format;
 virDomainXMLPrivateDataParseFunc  parse;
 };
@@ -2563,7 +2566,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr 
def);
 void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def);
 void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
 void

[libvirt] [PATCH 01/12] qemu: Introduce qemuDomainSecretInfo

Introduce a new private structure to hold qemu domain auth/secret data.
This will be stored in the qemuDomainDiskPrivate as a means to store the
auth and fetched secret data rather than generating during building of
the command line.

The initial changes will handle the current username and secret values
for rbd and iscsi disks (in their various forms). The rbd secret is
stored as a base64 encoded value, while the iscsi secret is stored as
a plain text value. Future changes will store encoded/encrypted secret
data as well as an initialization vector needed to be given to qemu
in order to decrypt the encoded password along with the domain masterKey.
The inital assumption will be that VIR_DOMAIN_SECRET_INFO_PLAIN is
being used.

Although it's expected that the cleanup of the secret data will be
done immediately after command line generation, reintroduce the object
dispose function qemuDomainDiskPrivateDispose to handle removing
memory associated with the structure for "normal" cleanup paths.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_domain.c | 32 +++-
 src/qemu/qemu_domain.h | 27 +++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 223154d..e5b7b9d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -720,7 +720,28 @@ qemuDomainMasterKeyCreate(qemuDomainObjPrivatePtr priv)
 }
 
 
+static void
+qemuDomainSecretPlainFree(qemuDomainSecretPlain secret)
+{
+VIR_FREE(secret.username);
+memset(secret.secret, 0, strlen(secret.secret));
+VIR_FREE(secret.secret);
+}
+
+
+static void
+qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo)
+{
+if (!*secinfo)
+return;
+
+qemuDomainSecretPlainFree((*secinfo)->s.plain);
+VIR_FREE(*secinfo);
+}
+
+
 static virClassPtr qemuDomainDiskPrivateClass;
+static void qemuDomainDiskPrivateDispose(void *obj);
 
 static int
 qemuDomainDiskPrivateOnceInit(void)
@@ -728,7 +749,7 @@ qemuDomainDiskPrivateOnceInit(void)
 qemuDomainDiskPrivateClass = virClassNew(virClassForObject(),
  "qemuDomainDiskPrivate",
  sizeof(qemuDomainDiskPrivate),
- NULL);
+ qemuDomainDiskPrivateDispose);
 if (!qemuDomainDiskPrivateClass)
 return -1;
 else
@@ -752,6 +773,15 @@ qemuDomainDiskPrivateNew(void)
 }
 
 
+static void
+qemuDomainDiskPrivateDispose(void *obj)
+{
+qemuDomainDiskPrivatePtr priv = obj;
+
+qemuDomainSecretInfoFree(>secinfo);
+}
+
+
 /* This is the old way of setting up per-domain directories */
 static int
 qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 80b6593..3538dca 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -223,6 +223,29 @@ struct _qemuDomainObjPrivate {
 size_t masterKeyLen;
 };
 
+/* Type of domain secret */
+typedef enum {
+VIR_DOMAIN_SECRET_INFO_PLAIN = 0,
+
+VIR_DOMAIN_SECRET_INFO_LAST
+} qemuDomainSecretInfoType;
+
+typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain;
+typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr;
+struct _qemuDomainSecretPlain {
+char *username;
+char *secret;
+};
+
+typedef struct _qemuDomainSecretInfo qemuDomainSecretInfo;
+typedef qemuDomainSecretInfo *qemuDomainSecretInfoPtr;
+struct _qemuDomainSecretInfo {
+int type;  /* qemuDomainSecretInfoType */
+union {
+qemuDomainSecretPlain plain;
+} s;
+};
+
 # define QEMU_DOMAIN_DISK_PRIVATE(disk)\
 ((qemuDomainDiskPrivatePtr) (disk)->privateData)
 
@@ -242,6 +265,10 @@ struct _qemuDomainDiskPrivate {
 bool blockJobSync; /* the block job needs synchronized termination */
 
 bool migrating; /* the disk is being migrated */
+
+/* for storage devices using auth/secret
+ * NB: *not* to be written to qemu domain object XML */
+qemuDomainSecretInfoPtr secinfo;
 };
 
 typedef enum {
-- 
2.5.5

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

[libvirt] [PATCH 00/12] Add IV Secret Object support

The first 5 patches handle changing the current mechanism of getting
the secret while building the command line into a mechanism where the
secret is built and stored as part of the disk or hostdev private data
and then parsed during command line generation.  The end result is to
remove the 'conn' parameter from the qemuBuildCommandLine.

The next 5 patches deal with some hotplug cleanup that was necessary
in order to "more easily" handle having a secret object as a possible
option instead of relying on the '-drive' string to hold the secret.

The last 2 patches are the resulting initialization vector patches.
They haven't been fully tested or vetted, but I figured getting something
"out there" for review would give me some more time to figure out a
proper test environment.  At the very least each of the first two
groups of 5 patches could be their own series - it's just that they
ended up being part of this code.


John Ferlan (12):
  qemu: Introduce qemuDomainSecretInfo
  qemu: Introduce qemuDomainSecretPrepare and Destroy
  qemu: Introduce qemuDomainHostdevPrivatePtr
  qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
  qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI
  qemu: hotplug: Assume support for -device for attach virtio disk
  qemu: hotplug: Adjust error path for attach virtio disk
  qemu: hotplug: Adjust error path for attach scsi disk
  qemu: hotplug: Adjust error path for attach hostdev scsi disk
  qemu: hotplug: Fix possible memory leak of props
  qemu: Introduce qemuDomainSecretIV
  qemu: Utilize qemu secret objects for SCSI/RBD auth/secret

 configure.ac  |   1 +
 src/conf/domain_conf.c|  28 ++-
 src/conf/domain_conf.h|   7 +-
 src/lxc/lxc_native.c  |   4 +-
 src/qemu/qemu_alias.c |  23 ++
 src/qemu/qemu_alias.h |   2 +
 src/qemu/qemu_command.c   | 321 ++
 src/qemu/qemu_command.h   |  20 +-
 src/qemu/qemu_domain.c| 508 +-
 src/qemu/qemu_domain.h|  81 ++-
 src/qemu/qemu_driver.c|  13 +-
 src/qemu/qemu_hotplug.c   | 309 -
 src/qemu/qemu_hotplug.h   |   4 +-
 src/qemu/qemu_parse_command.c |   4 +-
 src/qemu/qemu_process.c   |  13 +-
 src/vbox/vbox_common.c|   4 +-
 src/xenconfig/xen_common.c|   4 +-
 src/xenconfig/xen_sxpr.c  |   4 +-
 tests/virhostdevtest.c|   3 +-
 19 files changed, 1107 insertions(+), 246 deletions(-)

-- 
2.5.5

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

Re: [libvirt] [PATCH v2 09/10] vz: minor cleanup



On 07.04.2016 23:10, Maxim Nestratov wrote:
> remove unnecessary vzConnectClose prototype and make
> local structure vzDomainDefParserConfig be static
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_driver.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index 50da2fe..b3ce404 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -62,7 +62,6 @@ VIR_LOG_INIT("parallels.parallels_driver");
>  
>  #define PRLCTL  "prlctl"
>  
> -static int vzConnectClose(virConnectPtr conn);
>  static virClassPtr vzDriverConnClass;
>  
>  static virMutex vz_driver_lock;
> @@ -266,7 +265,7 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>  }
>  
>  
> -virDomainDefParserConfig vzDomainDefParserConfig = {
> +static virDomainDefParserConfig vzDomainDefParserConfig = {
>  .macPrefix = {0x42, 0x1C, 0x00},
>  .devicesPostParseCallback = vzDomainDeviceDefPostParse,
>  .domainPostParseCallback = vzDomainDefPostParse,
> 

ACK

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

Re: [libvirt] [PATCH 09/10] conf: Refactor virDomainDiskDefMirrorParse

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> Now that the mirror parsing code is not crammed in the main disk parser
> we can employ better coding style.
> ---
>  src/conf/domain_conf.c | 75 
> +-
>  1 file changed, 31 insertions(+), 44 deletions(-)

ACK

- Cole

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

Re: [libvirt] [PATCH 10/10] conf: extract disk geometry parsing code

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> ---
>  src/conf/domain_conf.c | 75 
> +++---
>  1 file changed, 46 insertions(+), 29 deletions(-)
> 

ACK

- Cole

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

Re: [libvirt] [PATCH 06/10] conf: virDomainDiskDefIotuneParse: simplfiy parsing

2016-04-12 Thread Ján Tomko

s/simplfiy/simplify/ in the commit summary.

Jan

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

Re: [libvirt] [PATCH v2 08/10] vz: remove vzDriverLock/Unlock function



On 07.04.2016 23:10, Maxim Nestratov wrote:
> We don't need them anymore as all pointers within vzDriver structure
> are not changed during the time it exists.
> Where we still need to synchronize we use virObjectLock/Unlock as far
> as vzDriver is lockable object.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_driver.c | 35 ---
>  src/vz/vz_utils.h  |  2 --
>  2 files changed, 37 deletions(-)
> 
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index dce7a87..50da2fe 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c

...

> @@ -1537,7 +1506,6 @@ vzConnectRegisterCloseCallback(virConnectPtr conn,
>  vzConnPtr privconn = conn->privateData;
>  int ret = -1;
>  
> -vzDriverLock(privconn);
>  if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != 
> NULL) {
>  virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("A close callback is already registered"));
> @@ -1549,7 +1517,6 @@ vzConnectRegisterCloseCallback(virConnectPtr conn,
>  ret = 0;
>  
>   cleanup:
> -vzDriverUnlock(privconn);
>  
>  return ret;
>  }
> @@ -1560,7 +1527,6 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, 
> virConnectCloseFunc cb)
>  vzConnPtr privconn = conn->privateData;
>  int ret = -1;
>  
> -vzDriverLock(privconn);
>  
>  if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != 
> cb) {
>  virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> @@ -1572,7 +1538,6 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, 
> virConnectCloseFunc cb)
>  ret = 0;
>  
>   cleanup:
> -vzDriverUnlock(privconn);
>  
>  return ret;
>  }

In this two particular functions we still need locking. The problem is that
close connect object API is not complete self-locking. We need to use
higher level lock because the check that register is possible and register
itself are not atomic. This was made intentionally to make register
function result type be void so that remote driver could be written
transactionally. On ther other hand we can not be client side driver
anymore and our only user is remote driver/daemon pair which is 
considered correct. So it's up to you.

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

Re: [libvirt] [PATCH 08/10] conf: disk: Split out parsing of disk mirror data

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> Changes are indentation and 'cleanup' label instead of 'error'.
> ---
>  src/conf/domain_conf.c | 192 
> ++---
>  1 file changed, 104 insertions(+), 88 deletions(-)
> 

ACK

- Cole

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

Re: [libvirt] [PATCH 07/10] conf: virDomainDiskDefIotuneParse: Report malformed number errors

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> Rest of the fields of the iotune data structure did not check for
> malformed integers. Use the previously defined macro to extract them
> which will simplify the code and add error reporting.
> ---
>  src/conf/domain_conf.c | 50 
> --
>  1 file changed, 8 insertions(+), 42 deletions(-)
> 

ACK... we need more of these cleanups, there's way too much boilerplate in the
codebase (he says as he just tried to add an extra 100 with setlocale error
checking :) )

- Cole

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

Re: [libvirt] [PATCH 06/10] conf: virDomainDiskDefIotuneParse: simplfiy parsing

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> Since the structure was pre-initialized to 0 we don't need to set every
> single member to 0 if it's not present in the XML. Additionally if we
> put the name of the field into the error message the code can be
> simplified using a macro to parse the members.
> ---
>  src/conf/domain_conf.c | 81 
> +-
>  1 file changed, 14 insertions(+), 67 deletions(-)
> 

Technically virXPathULongLong can thrown an error with -1 if one of the passed
values is NULL, but old code wasn't handling that, and virxml.c should
probably either drop those conditionals or use a separate error code for it.

ACK

- Cole

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

Re: [libvirt] [PATCH 05/10] conf: disk: Remove error label from virDomainDiskDefIotuneParse

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> Since this function isn't doing any cleanup, the label is not necessary.
> ---
>  src/conf/domain_conf.c | 23 ++-
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 

ACK

- Cole

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

Re: [libvirt] [PATCH v2 07/10] vz: implement connectGetSysinfo hypervisor callback



On 07.04.2016 23:09, Maxim Nestratov wrote:
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_driver.c | 25 +
>  src/vz/vz_utils.h  |  1 +
>  2 files changed, 26 insertions(+)

ACK

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

Re: [libvirt] [PATCH 04/10] conf: disk: Extract iotune parsing into a separate func

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> ---
>  src/conf/domain_conf.c | 312 
> ++---
>  1 file changed, 163 insertions(+), 149 deletions(-)
> 

ACK

- Cole

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

Re: [libvirt] [PATCH 0/7] Add notification for memory hot-unplug failure

On 04/12/2016 09:53 AM, Peter Krempa wrote:
> On Tue, Apr 05, 2016 at 17:09:16 +0200, Peter Krempa wrote:
>> Add a new libvirt event VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED that will
>> handle failure to unplug a device if we are certain that it will not be 
>> unplugged
>> and wire up the ACPI_DEVICE_OST qemu event that is emitted on memory 
>> hotunplug
>> failure so that the event is propagated and the API fails in such case.
>>
>> Along with that a few refactors were necessary.
> 
> Ping? Anybody brave enough to look at this? Thanks.
> 

I'll review it today if no one else gets to it, I just started looking at your
disk XML refactoring bits too

- Cole

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

Re: [libvirt] [PATCH v2 04/10] vz: add Hypervisor prefix to vz and parallels Driver structures



On 07.04.2016 23:09, Maxim Nestratov wrote:
> ---
>  src/vz/vz_driver.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index 9de88cd..f2bbf1e 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -1491,7 +1491,7 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, 
> virConnectCloseFunc cb)
>  return ret;
>  }
>  
> -static virHypervisorDriver vzDriver = {
> +static virHypervisorDriver vzHypervisorDriver = {
>  .name = "vz",
>  .connectOpen = vzConnectOpen,/* 0.10.0 */
>  .connectClose = vzConnectClose,  /* 0.10.0 */
> @@ -1558,11 +1558,11 @@ static virHypervisorDriver vzDriver = {
>  };
>  
>  static virConnectDriver vzConnectDriver = {
> -.hypervisorDriver = ,
> +.hypervisorDriver = ,
>  };
>  
>  /* Parallels domain type backward compatibility*/
> -static virHypervisorDriver parallelsDriver;
> +static virHypervisorDriver parallelsHypervisorDriver;
>  static virConnectDriver parallelsConnectDriver;
>  
>  /**
> @@ -1584,10 +1584,10 @@ vzRegister(void)
>  VIR_FREE(prlctl_path);
>  
>  /* Backward compatibility with Parallels domain type */
> -parallelsDriver = vzDriver;
> -parallelsDriver.name = "Parallels";
> +parallelsHypervisorDriver = vzHypervisorDriver;
> +parallelsHypervisorDriver.name = "Parallels";
>  parallelsConnectDriver = vzConnectDriver;
> -parallelsConnectDriver.hypervisorDriver = 
> +parallelsConnectDriver.hypervisorDriver = 
>  if (virRegisterConnectDriver(, false) < 0)
>  return -1;
>  
> 

ACK


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

Re: [libvirt] [PATCH] Be consistent with setlocale error handling

On 04/12/2016 10:10 AM, Daniel P. Berrange wrote:
> On Tue, Apr 12, 2016 at 10:00:48AM -0400, Cole Robinson wrote:
>> Take setlocale/gettext error handling pattern from tools/virsh-*
>> and use it in all the other standalone binaries. The changes are
>>
>> * Ignore setlocale errors. virsh has done this forever, presumably for
>>   good reason. This has been partially responsible for some bug reports:
>>
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1312688
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1026514
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1016158
>>
>> * Report the failed function name
>> * Report strerror
>> ---
>>  daemon/libvirtd.c | 20 
>>  src/locking/lock_daemon.c | 20 
>>  src/locking/sanlock_helper.c  | 16 
>>  src/logging/log_daemon.c  | 20 
>>  src/lxc/lxc_controller.c  | 20 
>>  src/network/leaseshelper.c| 16 
>>  src/security/virt-aa-helper.c | 16 
>>  src/storage/parthelper.c  | 16 
>>  src/util/iohelper.c   | 16 
>>  9 files changed, 124 insertions(+), 36 deletions(-)
>>
>> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
>> index 3d38a46..9488950 100644
>> --- a/daemon/libvirtd.c
>> +++ b/daemon/libvirtd.c
>> @@ -1172,10 +1172,22 @@ int main(int argc, char **argv) {
>>  {0, 0, 0, 0}
>>  };
>>  
>> -if (setlocale(LC_ALL, "") == NULL ||
>> -bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
>> -textdomain(PACKAGE) == NULL ||
>> -virInitialize() < 0) {
>> +if (!setlocale(LC_ALL, "")) {
>> +perror("setlocale");
>> +/* failure to setup locale is not fatal */
>> +}
>> +
>> +if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
>> +perror("bindtextdomain");
>> +exit(EXIT_FAILURE);
>> +}
>> +
>> +if (!textdomain(PACKAGE)) {
>> +perror("textdomain");
>> +exit(EXIT_FAILURE);
>> +}
>> +
>> +if (virInitialize() < 0) {
>>  fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
>>  exit(EXIT_FAILURE);
>>  }
> 
> Instead of repeating this, how about we add  src/util/virgettext.h and
> then have
> 
> 
>   int virGettextInit(const char *package, const char *localedir) {
> if (!setlocale(LC_ALL, "")) {
> perror("setlocale");
> /* failure to setup locale is not fatal */
> }
> 
> if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
> perror("bindtextdomain");
> return -1;
> }
> 
> if (!textdomain(PACKAGE)) {
> perror("textdomain");
> return -1;
> }
> return 0;
>   }
> 
> 
> And in each app we can just do
> 
>if (virGettextInit(PACKAGE, LOCALEDIR) < 0)
>   exit(EXIT_FAILURE);
> 
> Regards,
> Daniel
> 

Good idea, I'll send a v2

Thanks,
Cole

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

Re: [libvirt] [PATCH] Be consistent with setlocale error handling

2016-04-12 Thread Daniel P. Berrange

On Tue, Apr 12, 2016 at 10:00:48AM -0400, Cole Robinson wrote:
> Take setlocale/gettext error handling pattern from tools/virsh-*
> and use it in all the other standalone binaries. The changes are
> 
> * Ignore setlocale errors. virsh has done this forever, presumably for
>   good reason. This has been partially responsible for some bug reports:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1312688
>   https://bugzilla.redhat.com/show_bug.cgi?id=1026514
>   https://bugzilla.redhat.com/show_bug.cgi?id=1016158
> 
> * Report the failed function name
> * Report strerror
> ---
>  daemon/libvirtd.c | 20 
>  src/locking/lock_daemon.c | 20 
>  src/locking/sanlock_helper.c  | 16 
>  src/logging/log_daemon.c  | 20 
>  src/lxc/lxc_controller.c  | 20 
>  src/network/leaseshelper.c| 16 
>  src/security/virt-aa-helper.c | 16 
>  src/storage/parthelper.c  | 16 
>  src/util/iohelper.c   | 16 
>  9 files changed, 124 insertions(+), 36 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 3d38a46..9488950 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1172,10 +1172,22 @@ int main(int argc, char **argv) {
>  {0, 0, 0, 0}
>  };
>  
> -if (setlocale(LC_ALL, "") == NULL ||
> -bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
> -textdomain(PACKAGE) == NULL ||
> -virInitialize() < 0) {
> +if (!setlocale(LC_ALL, "")) {
> +perror("setlocale");
> +/* failure to setup locale is not fatal */
> +}
> +
> +if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
> +perror("bindtextdomain");
> +exit(EXIT_FAILURE);
> +}
> +
> +if (!textdomain(PACKAGE)) {
> +perror("textdomain");
> +exit(EXIT_FAILURE);
> +}
> +
> +if (virInitialize() < 0) {
>  fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
>  exit(EXIT_FAILURE);
>  }

Instead of repeating this, how about we add  src/util/virgettext.h and
then have


  int virGettextInit(const char *package, const char *localedir) {
if (!setlocale(LC_ALL, "")) {
perror("setlocale");
/* failure to setup locale is not fatal */
}

if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
perror("bindtextdomain");
return -1;
}

if (!textdomain(PACKAGE)) {
perror("textdomain");
return -1;
}
return 0;
  }


And in each app we can just do

   if (virGettextInit(PACKAGE, LOCALEDIR) < 0)
  exit(EXIT_FAILURE);

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 03/10] conf: disk: Remove one unnecessary level of indentation

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> Also simplify the code by switching to a for loop.
> ---
>  src/conf/domain_conf.c | 737 
> -
>  1 file changed, 368 insertions(+), 369 deletions(-)
> 

git show -w makes it clear:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fb5d327..f691174 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6760,9 +6760,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 rawio = virXMLPropString(node, "rawio");
 sgio = virXMLPropString(node, "sgio");

-cur = node->children;
-while (cur != NULL) {
-if (cur->type == XML_ELEMENT_NODE) {
+for (cur = node->children; cur != NULL; cur = cur->next) {
+if (cur->type != XML_ELEMENT_NODE)
+continue;
+
 if (!source && xmlStrEqual(cur->name, BAD_CAST "source")) {
 sourceNode = cur;

@@ -7176,8 +7177,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 /* boot is parsed as part of virDomainDeviceInfoParseXML */
 }
 }
-cur = cur->next;
-}

 /* Disk volume types will have authentication information handled in
  * virStorageTranslateDiskSourcePool

ACK

- Cole

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

Re: [libvirt] [PATCH 02/10] conf: disk: Don't initialize fields allocated by calloc

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> All the fields were initialized to 0.
> ---
>  src/conf/domain_conf.c | 8 
>  1 file changed, 8 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0320691..aa96bac 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6737,14 +6737,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>  if (!(def = virDomainDiskDefNew(xmlopt)))
>  return NULL;
> 
> -def->geometry.cylinders = 0;
> -def->geometry.heads = 0;
> -def->geometry.sectors = 0;
> -def->geometry.trans = VIR_DOMAIN_DISK_TRANS_DEFAULT;
> -
> -def->blockio.logical_block_size = 0;
> -def->blockio.physical_block_size = 0;
> -
>  ctxt->node = node;
> 
>  type = virXMLPropString(node, "type");
> 

ACK

- Cole

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

[libvirt] [PATCH] Libvirt: virTypedParamsValidate: Fix detection of multiple parameters

2016-04-12 Thread Jason J. Herne

virTypedParamsValidate currently uses an index based check to find
duplicate parameters. This check does not work. Consider the following
simple example:

We have only 2 keys
A  (multiples allowed)
B  (multiples NOT allowed)

We are given the following list of parameters to check:
A
A
B

If you work through the validation loop you will see that our last iteration
through the loop has i=2 and j=1. In this case, i > j and keys[j].value.i will
indicate that multiples are not allowed. Both conditionals are satisfied so
an incorrect error will be given: "parameter '%s' occurs multiple times"

This patch replaces the index based check with code that remembers
the name of the last parameter seen and only triggers the error case if
the current parameter name equals the last one. This works because the
list is sorted and duplicate parameters will be grouped together.

In reality, we hit this bug while using selective block migration to migrate
a guest with 5 disks. 5 was apparently just the right number to push i > j
and hit this bug.

virsh migrate --live guestname --copy-storage-all
  --migrate-disks vdb,vdc,vdd,vde,vdf
  qemu+ssh://dsthost/system

Signed-off-by: Jason J. Herne 
Reviewed-by: Eric Farman 
---
 src/util/virtypedparam.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index 138fc64..23109e1 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -66,7 +66,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int 
nparams, ...)
 va_list ap;
 int ret = -1;
 size_t i, j;
-const char *name;
+const char *name, *last_name = NULL;
 int type;
 size_t nkeys = 0, nkeysalloc = 0;
 virTypedParameterPtr sorted = NULL, keys = NULL;
@@ -106,7 +106,8 @@ virTypedParamsValidate(virTypedParameterPtr params, int 
nparams, ...)
 if (STRNEQ(sorted[i].field, keys[j].field)) {
 j++;
 } else {
-if (i > j && !(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) {
+if (STREQ_NULLABLE(last_name, sorted[i].field) &&
+!(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) {
 virReportError(VIR_ERR_INVALID_ARG,
_("parameter '%s' occurs multiple times"),
sorted[i].field);
@@ -125,6 +126,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int 
nparams, ...)
virTypedParameterTypeToString(keys[j].type));
 goto cleanup;
 }
+last_name = sorted[i].field;
 i++;
 }
 }
-- 
1.9.1

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

Re: [libvirt] [PATCH 01/10] util: Rename and move virStrIsPrint to virStringIsPrintable

On 04/12/2016 09:55 AM, Peter Krempa wrote:
> ---
>  src/conf/domain_conf.c   |  4 ++--
>  src/libvirt_private.syms |  2 +-
>  src/util/virstring.c | 18 ++
>  src/util/virstring.h |  2 ++
>  src/util/virutil.c   | 12 
>  src/util/virutil.h   |  2 --
>  6 files changed, 23 insertions(+), 17 deletions(-)
> 

ACK

- Cole

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

[libvirt] [PATCH 01/10] util: Rename and move virStrIsPrint to virStringIsPrintable

---
 src/conf/domain_conf.c   |  4 ++--
 src/libvirt_private.syms |  2 +-
 src/util/virstring.c | 18 ++
 src/util/virstring.h |  2 ++
 src/util/virutil.c   | 12 
 src/util/virutil.h   |  2 --
 6 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d2cf8d5..0320691 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7155,7 +7155,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 goto error;
 }

-if (!virStrIsPrint(vendor)) {
+if (!virStringIsPrintable(vendor)) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("disk vendor is not printable string"));
 goto error;
@@ -7170,7 +7170,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 goto error;
 }

-if (!virStrIsPrint(product)) {
+if (!virStringIsPrintable(product)) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("disk product is not printable string"));
 goto error;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 068bc00..a79d85e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2265,6 +2265,7 @@ virStringFreeListCount;
 virStringGetFirstWithPrefix;
 virStringHasControlChars;
 virStringIsEmpty;
+virStringIsPrintable;
 virStringJoin;
 virStringListLength;
 virStringReplace;
@@ -2478,7 +2479,6 @@ virSetNonBlock;
 virSetSockReuseAddr;
 virSetUIDGID;
 virSetUIDGIDWithCaps;
-virStrIsPrint;
 virTristateBoolTypeFromString;
 virTristateBoolTypeToString;
 virTristateSwitchTypeFromString;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 2d7fbf3..384e3f7 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1048,3 +1048,21 @@ virStringToUpper(char **dst, const char *src)
 *dst = cap;
 return 1;
 }
+
+
+/**
+ * virStrIsPrintable:
+ *
+ * Returns true @str contains only printable characters.
+ */
+bool
+virStringIsPrintable(const char *str)
+{
+size_t i;
+
+for (i = 0; str[i]; i++)
+if (!c_isprint(str[i]))
+return false;
+
+return true;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 16ed3b2..fd2868a 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -275,4 +275,6 @@ void virStringStripIPv6Brackets(char *str);
 bool virStringHasControlChars(const char *str);
 void virStringStripControlChars(char *str);

+bool virStringIsPrintable(const char *str);
+
 #endif /* __VIR_STRING_H__ */
diff --git a/src/util/virutil.c b/src/util/virutil.c
index b401f8d..1b46ea1 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1600,18 +1600,6 @@ virValidateWWN(const char *wwn)
 return true;
 }

-bool
-virStrIsPrint(const char *str)
-{
-size_t i;
-
-for (i = 0; str[i]; i++)
-if (!c_isprint(str[i]))
-return false;
-
-return true;
-}
-
 #if defined(major) && defined(minor)
 int
 virGetDeviceID(const char *path, int *maj, int *min)
diff --git a/src/util/virutil.h b/src/util/virutil.h
index b121de0..1e51a25 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -152,8 +152,6 @@ bool virIsDevMapperDevice(const char *dev_name) 
ATTRIBUTE_NONNULL(1);

 bool virValidateWWN(const char *wwn);

-bool virStrIsPrint(const char *str);
-
 int virGetDeviceID(const char *path,
int *maj,
int *min);
-- 
2.8.0

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

Re: [libvirt] [libvirt-users] Libvirtd running as root tries to access oneadmin (OpenNebula) NFS mount but throws: error: can’t canonicalize path


On Mon, Apr 11, 2016 at 08:02:04PM -0400, TomK wrote:

Hey All,

Wondering if anyone had any suggestions on this topic?



The only thing I can come up with is:
'/var/lib/one//datastores/0/38/disk.1': Permission denied

... that don't have access to that file.  Could you elaborate on that?

I think it's either:

a) you are running the domain as root or

b) we don't use the domain's uid/gid to canonicalize the path.

But if read access is enough for canonicalizing that path, I think the
problem is purely with permissions.


Cheers,
Tom K.
-

Living on earth is expensive, but it includes a free trip around the sun.

On 4/9/2016 11:08 AM, TomK wrote:

Adding in libvir-list.

Cheers,
Tom K.
-

Living on earth is expensive, but it includes a free trip around the sun.

On 4/7/2016 7:32 PM, TomK wrote:

Hey All,

I've an issue where libvirtd tries to access an NFS mount but errors
out with: can't canonicalize path '/var/lib/one//datastores/0 .  The
unprevilidged user is able to read/write fine to the share.
root_squash is used and for security reasons no_root_squash cannot be
used.

On the controller and node SELinux is disabled.

[oneadmin@mdskvm-p01 ~]$ virsh -d 1 --connect qemu:///system create
/var/lib/one//datastores/0/38/deployment.0
create: file(optdata): /var/lib/one//datastores/0/38/deployment.0
error: Failed to create domain from
/var/lib/one//datastores/0/38/deployment.0
error: can't canonicalize path
'/var/lib/one//datastores/0/38/disk.1': Permission denied

I added some debug flags to get more info and added -x to the deploy
script. Closest I get to more details is this:

2016-04-06 04:15:35.945+: 14072: debug :
virStorageFileBackendFileInit:1441 : initializing FS storage file
0x7f6aa4009000 (file:/var/lib/one//datastores/0/38/disk.1)[9869:9869]
2016-04-06 04:15:35.954+: 14072: error :
virStorageFileBackendFileGetUniqueIdentifier:1523 : can't
canonicalize path '/var/lib/one//datastores/0/38/disk.1':

https://www.redhat.com/archives/libvir-list/2014-May/msg00194.html

Comment is: "The current implementation works for local
storage only and returns the canonical path of the volume."

But it seems the logic is applied to NFS mounts. Perhaps it shouldn't
be?  Anyway to get around this problem?  This is CentOS 7 .

Cheers,
Tom K.
-

Living on earth is expensive, but it includes a free trip around the
sun.

___
libvirt-users mailing list
libvirt-us...@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users


--
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: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 04/10] conf: disk: Extract iotune parsing into a separate func

---
 src/conf/domain_conf.c | 312 ++---
 1 file changed, 163 insertions(+), 149 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f359e95..c2ac8d6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6676,6 +6676,168 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
 }


+static int
+virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
+xmlXPathContextPtr ctxt)
+{
+int ret;
+
+ret = virXPathULongLong("string(./iotune/total_bytes_sec)",
+ctxt,
+>blkdeviotune.total_bytes_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("total throughput limit must be an integer"));
+goto error;
+} else if (ret < 0) {
+def->blkdeviotune.total_bytes_sec = 0;
+}
+
+ret = virXPathULongLong("string(./iotune/read_bytes_sec)",
+ctxt,
+>blkdeviotune.read_bytes_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("read throughput limit must be an integer"));
+goto error;
+} else if (ret < 0) {
+def->blkdeviotune.read_bytes_sec = 0;
+}
+
+ret = virXPathULongLong("string(./iotune/write_bytes_sec)",
+ctxt,
+>blkdeviotune.write_bytes_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("write throughput limit must be an integer"));
+goto error;
+} else if (ret < 0) {
+def->blkdeviotune.write_bytes_sec = 0;
+}
+
+ret = virXPathULongLong("string(./iotune/total_iops_sec)",
+ctxt,
+>blkdeviotune.total_iops_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("total I/O operations limit must be an integer"));
+goto error;
+} else if (ret < 0) {
+def->blkdeviotune.total_iops_sec = 0;
+}
+
+ret = virXPathULongLong("string(./iotune/read_iops_sec)",
+ctxt,
+>blkdeviotune.read_iops_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("read I/O operations limit must be an integer"));
+goto error;
+} else if (ret < 0) {
+def->blkdeviotune.read_iops_sec = 0;
+}
+
+ret = virXPathULongLong("string(./iotune/write_iops_sec)",
+ctxt,
+>blkdeviotune.write_iops_sec);
+if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("write I/O operations limit must be an integer"));
+goto error;
+} else if (ret < 0) {
+def->blkdeviotune.write_iops_sec = 0;
+}
+
+if (virXPathULongLong("string(./iotune/total_bytes_sec_max)",
+  ctxt,
+  >blkdeviotune.total_bytes_sec_max) < 0) {
+def->blkdeviotune.total_bytes_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/read_bytes_sec_max)",
+  ctxt,
+  >blkdeviotune.read_bytes_sec_max) < 0) {
+def->blkdeviotune.read_bytes_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/write_bytes_sec_max)",
+  ctxt,
+  >blkdeviotune.write_bytes_sec_max) < 0) {
+def->blkdeviotune.write_bytes_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/total_iops_sec_max)",
+  ctxt,
+  >blkdeviotune.total_iops_sec_max) < 0) {
+def->blkdeviotune.total_iops_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/read_iops_sec_max)",
+  ctxt,
+  >blkdeviotune.read_iops_sec_max) < 0) {
+def->blkdeviotune.read_iops_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/write_iops_sec_max)",
+  ctxt,
+  >blkdeviotune.write_iops_sec_max) < 0) {
+def->blkdeviotune.write_iops_sec_max = 0;
+}
+
+if (virXPathULongLong("string(./iotune/size_iops_sec)",
+  ctxt,
+  >blkdeviotune.size_iops_sec) < 0) {
+def->blkdeviotune.size_iops_sec = 0;
+}
+
+
+if ((def->blkdeviotune.total_bytes_sec &&
+ def->blkdeviotune.read_bytes_sec) ||
+(def->blkdeviotune.total_bytes_sec &&
+ def->blkdeviotune.write_bytes_sec)) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("total and read/write bytes_sec "
+ "cannot be set at the same time"));
+goto error;
+}
+
+if ((def->blkdeviotune.total_iops_sec &&
+

Re: [libvirt] [PATCH v2 03/10] vz: remove drivername field from vzConn structure



On 07.04.2016 23:09, Maxim Nestratov wrote:
> No need to remember connection name and have corresponding
> domain type to keep backward compatibility with former
> 'parallels' driver. It is enough to be able to accept 'parallels'
> uri and domain types.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_driver.c | 2 --
>  src/vz/vz_utils.c  | 5 +
>  src/vz/vz_utils.h  | 1 -
>  3 files changed, 1 insertion(+), 7 deletions(-)


ACK

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

[libvirt] [PATCH] Be consistent with setlocale error handling

Take setlocale/gettext error handling pattern from tools/virsh-*
and use it in all the other standalone binaries. The changes are

* Ignore setlocale errors. virsh has done this forever, presumably for
  good reason. This has been partially responsible for some bug reports:

  https://bugzilla.redhat.com/show_bug.cgi?id=1312688
  https://bugzilla.redhat.com/show_bug.cgi?id=1026514
  https://bugzilla.redhat.com/show_bug.cgi?id=1016158

* Report the failed function name
* Report strerror
---
 daemon/libvirtd.c | 20 
 src/locking/lock_daemon.c | 20 
 src/locking/sanlock_helper.c  | 16 
 src/logging/log_daemon.c  | 20 
 src/lxc/lxc_controller.c  | 20 
 src/network/leaseshelper.c| 16 
 src/security/virt-aa-helper.c | 16 
 src/storage/parthelper.c  | 16 
 src/util/iohelper.c   | 16 
 9 files changed, 124 insertions(+), 36 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 3d38a46..9488950 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1172,10 +1172,22 @@ int main(int argc, char **argv) {
 {0, 0, 0, 0}
 };
 
-if (setlocale(LC_ALL, "") == NULL ||
-bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-textdomain(PACKAGE) == NULL ||
-virInitialize() < 0) {
+if (!setlocale(LC_ALL, "")) {
+perror("setlocale");
+/* failure to setup locale is not fatal */
+}
+
+if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+perror("bindtextdomain");
+exit(EXIT_FAILURE);
+}
+
+if (!textdomain(PACKAGE)) {
+perror("textdomain");
+exit(EXIT_FAILURE);
+}
+
+if (virInitialize() < 0) {
 fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
 exit(EXIT_FAILURE);
 }
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 973e691..fffbe1d 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1179,10 +1179,22 @@ int main(int argc, char **argv) {
 
 privileged = geteuid() == 0;
 
-if (setlocale(LC_ALL, "") == NULL ||
-bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-textdomain(PACKAGE) == NULL ||
-virThreadInitialize() < 0 ||
+if (!setlocale(LC_ALL, "")) {
+perror("setlocale");
+/* failure to setup locale is not fatal */
+}
+
+if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+perror("bindtextdomain");
+exit(EXIT_FAILURE);
+}
+
+if (!textdomain(PACKAGE)) {
+perror("textdomain");
+exit(EXIT_FAILURE);
+}
+
+if (virThreadInitialize() < 0 ||
 virErrorInitialize() < 0) {
 fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
 exit(EXIT_FAILURE);
diff --git a/src/locking/sanlock_helper.c b/src/locking/sanlock_helper.c
index d8d294f..6b17fce 100644
--- a/src/locking/sanlock_helper.c
+++ b/src/locking/sanlock_helper.c
@@ -70,10 +70,18 @@ main(int argc, char **argv)
 .cb = authCallback,
 };
 
-if (setlocale(LC_ALL, "") == NULL ||
-bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-textdomain(PACKAGE) == NULL) {
-fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
+if (!setlocale(LC_ALL, "")) {
+perror("setlocale");
+/* failure to setup locale is not fatal */
+}
+
+if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+perror("bindtextdomain");
+exit(EXIT_FAILURE);
+}
+
+if (!textdomain(PACKAGE)) {
+perror("textdomain");
 exit(EXIT_FAILURE);
 }
 
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index f674cbd..8a0de22 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -936,10 +936,22 @@ int main(int argc, char **argv) {
 
 privileged = geteuid() == 0;
 
-if (setlocale(LC_ALL, "") == NULL ||
-bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
-textdomain(PACKAGE) == NULL ||
-virThreadInitialize() < 0 ||
+if (!setlocale(LC_ALL, "")) {
+perror("setlocale");
+/* failure to setup locale is not fatal */
+}
+
+if (!bindtextdomain(PACKAGE, LOCALEDIR)) {
+perror("bindtextdomain");
+exit(EXIT_FAILURE);
+}
+
+if (!textdomain(PACKAGE)) {
+perror("textdomain");
+exit(EXIT_FAILURE);
+}
+
+if (virThreadInitialize() < 0 ||
 virErrorInitialize() < 0) {
 fprintf(stderr, _("%s: initialization failed\n"), argv[0]);
 exit(EXIT_FAILURE);
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 8b5ec4c..612c0d7 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -2505,10 +2505,22 @@ int main(int argc, char *argv[])
 for (i = 0; i < VIR_LXC_DOMAIN_NAMESPACE_LAST; i++)
 ns_fd[i] = -1;
 
-if (setlocale(LC_ALL, "") == NULL ||
-

Re: [libvirt] [PATCH] tests: fix xen-related tests

2016-04-12 Thread Pavel Hrdina

On Tue, Apr 12, 2016 at 03:04:07PM +0200, Ján Tomko wrote:
> My commit 6879be4 moved the addition of the implicit video device
> from the XML parser to the PostParse function, but did not regenerate
> all the tests.
> ---

ACK, Pavel

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

[libvirt] [PATCH] vz: correct iomode check

2016-04-12 Thread Mikhail Feoktistov

Virtuozzo hypervisor supports native iomode.
So we should allow to add disk with iomode "native" or "default".
---
 src/vz/vz_utils.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index fed48a5..a89cf73 100644
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -332,9 +332,10 @@ vzCheckDiskUnsupportedParams(virDomainDiskDefPtr disk)
 return -1;
 }
 
-if (disk->iomode) {
+if (disk->iomode != VIR_DOMAIN_DISK_IO_DEFAULT &&
+disk->iomode != VIR_DOMAIN_DISK_IO_NATIVE) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Setting disk io mode is not "
+   _("Only native iomode is "
  "supported by vz driver."));
 return -1;
 }
-- 
1.8.3.1

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

[libvirt] [PATCH 07/10] conf: virDomainDiskDefIotuneParse: Report malformed number errors

Rest of the fields of the iotune data structure did not check for
malformed integers. Use the previously defined macro to extract them
which will simplify the code and add error reporting.
---
 src/conf/domain_conf.c | 50 --
 1 file changed, 8 insertions(+), 42 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3c066b4..64561df 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6694,48 +6694,14 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 PARSE_IOTUNE(read_iops_sec);
 PARSE_IOTUNE(write_iops_sec);

-if (virXPathULongLong("string(./iotune/total_bytes_sec_max)",
-  ctxt,
-  >blkdeviotune.total_bytes_sec_max) < 0) {
-def->blkdeviotune.total_bytes_sec_max = 0;
-}
-
-if (virXPathULongLong("string(./iotune/read_bytes_sec_max)",
-  ctxt,
-  >blkdeviotune.read_bytes_sec_max) < 0) {
-def->blkdeviotune.read_bytes_sec_max = 0;
-}
-
-if (virXPathULongLong("string(./iotune/write_bytes_sec_max)",
-  ctxt,
-  >blkdeviotune.write_bytes_sec_max) < 0) {
-def->blkdeviotune.write_bytes_sec_max = 0;
-}
-
-if (virXPathULongLong("string(./iotune/total_iops_sec_max)",
-  ctxt,
-  >blkdeviotune.total_iops_sec_max) < 0) {
-def->blkdeviotune.total_iops_sec_max = 0;
-}
-
-if (virXPathULongLong("string(./iotune/read_iops_sec_max)",
-  ctxt,
-  >blkdeviotune.read_iops_sec_max) < 0) {
-def->blkdeviotune.read_iops_sec_max = 0;
-}
-
-if (virXPathULongLong("string(./iotune/write_iops_sec_max)",
-  ctxt,
-  >blkdeviotune.write_iops_sec_max) < 0) {
-def->blkdeviotune.write_iops_sec_max = 0;
-}
-
-if (virXPathULongLong("string(./iotune/size_iops_sec)",
-  ctxt,
-  >blkdeviotune.size_iops_sec) < 0) {
-def->blkdeviotune.size_iops_sec = 0;
-}
-
+PARSE_IOTUNE(total_bytes_sec_max);
+PARSE_IOTUNE(read_bytes_sec_max);
+PARSE_IOTUNE(write_bytes_sec_max);
+PARSE_IOTUNE(total_iops_sec_max);
+PARSE_IOTUNE(read_iops_sec_max);
+PARSE_IOTUNE(write_iops_sec_max);
+
+PARSE_IOTUNE(size_iops_sec);

 if ((def->blkdeviotune.total_bytes_sec &&
  def->blkdeviotune.read_bytes_sec) ||
-- 
2.8.0

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

[libvirt] [PATCH 02/10] conf: disk: Don't initialize fields allocated by calloc

All the fields were initialized to 0.
---
 src/conf/domain_conf.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0320691..aa96bac 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6737,14 +6737,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 if (!(def = virDomainDiskDefNew(xmlopt)))
 return NULL;

-def->geometry.cylinders = 0;
-def->geometry.heads = 0;
-def->geometry.sectors = 0;
-def->geometry.trans = VIR_DOMAIN_DISK_TRANS_DEFAULT;
-
-def->blockio.logical_block_size = 0;
-def->blockio.physical_block_size = 0;
-
 ctxt->node = node;

 type = virXMLPropString(node, "type");
-- 
2.8.0

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

[libvirt] [PATCH 06/10] conf: virDomainDiskDefIotuneParse: simplfiy parsing

Since the structure was pre-initialized to 0 we don't need to set every
single member to 0 if it's not present in the XML. Additionally if we
put the name of the field into the error message the code can be
simplified using a macro to parse the members.
---
 src/conf/domain_conf.c | 81 +-
 1 file changed, 14 insertions(+), 67 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ac464d3..3c066b4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6675,78 +6675,24 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
 return ret;
 }

+#define PARSE_IOTUNE(val)  
\
+if (virXPathULongLong("string(./iotune/" #val ")", 
\
+  ctxt, >blkdeviotune.val) == -2) {   
\
+virReportError(VIR_ERR_XML_ERROR,  
\
+   _("disk iotune field '%s' must be an integer"), #val);  
\
+return -1; 
\
+}

 static int
 virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 xmlXPathContextPtr ctxt)
 {
-int ret;
-
-ret = virXPathULongLong("string(./iotune/total_bytes_sec)",
-ctxt,
->blkdeviotune.total_bytes_sec);
-if (ret == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("total throughput limit must be an integer"));
-return -1;
-} else if (ret < 0) {
-def->blkdeviotune.total_bytes_sec = 0;
-}
-
-ret = virXPathULongLong("string(./iotune/read_bytes_sec)",
-ctxt,
->blkdeviotune.read_bytes_sec);
-if (ret == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("read throughput limit must be an integer"));
-return -1;
-} else if (ret < 0) {
-def->blkdeviotune.read_bytes_sec = 0;
-}
-
-ret = virXPathULongLong("string(./iotune/write_bytes_sec)",
-ctxt,
->blkdeviotune.write_bytes_sec);
-if (ret == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("write throughput limit must be an integer"));
-return -1;
-} else if (ret < 0) {
-def->blkdeviotune.write_bytes_sec = 0;
-}
-
-ret = virXPathULongLong("string(./iotune/total_iops_sec)",
-ctxt,
->blkdeviotune.total_iops_sec);
-if (ret == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("total I/O operations limit must be an integer"));
-return -1;
-} else if (ret < 0) {
-def->blkdeviotune.total_iops_sec = 0;
-}
-
-ret = virXPathULongLong("string(./iotune/read_iops_sec)",
-ctxt,
->blkdeviotune.read_iops_sec);
-if (ret == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("read I/O operations limit must be an integer"));
-return -1;
-} else if (ret < 0) {
-def->blkdeviotune.read_iops_sec = 0;
-}
-
-ret = virXPathULongLong("string(./iotune/write_iops_sec)",
-ctxt,
->blkdeviotune.write_iops_sec);
-if (ret == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("write I/O operations limit must be an integer"));
-return -1;
-} else if (ret < 0) {
-def->blkdeviotune.write_iops_sec = 0;
-}
+PARSE_IOTUNE(total_bytes_sec);
+PARSE_IOTUNE(read_bytes_sec);
+PARSE_IOTUNE(write_bytes_sec);
+PARSE_IOTUNE(total_iops_sec);
+PARSE_IOTUNE(read_iops_sec);
+PARSE_IOTUNE(write_iops_sec);

 if (virXPathULongLong("string(./iotune/total_bytes_sec_max)",
   ctxt,
@@ -6833,6 +6779,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,

 return 0;
 }
+#undef PARSE_IOTUNE


 #define VENDOR_LEN  8
-- 
2.8.0

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

[libvirt] [PATCH 09/10] conf: Refactor virDomainDiskDefMirrorParse

Now that the mirror parsing code is not crammed in the main disk parser
we can employ better coding style.
---
 src/conf/domain_conf.c | 75 +-
 1 file changed, 31 insertions(+), 44 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 802a6fe..e64471d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6753,41 +6753,44 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
 xmlNodePtr cur,
 xmlXPathContextPtr ctxt)
 {
+xmlNodePtr mirrorNode;
 char *mirrorFormat = NULL;
 char *mirrorType = NULL;
-char *ready;
-char *blockJob;
+char *ready = NULL;
+char *blockJob = NULL;
 int ret = -1;

 if (VIR_ALLOC(def->mirror) < 0)
 goto cleanup;

-blockJob = virXMLPropString(cur, "job");
-if (blockJob) {
-def->mirrorJob = virDomainBlockJobTypeFromString(blockJob);
-if (def->mirrorJob <= 0) {
+if ((blockJob = virXMLPropString(cur, "job"))) {
+if ((def->mirrorJob = virDomainBlockJobTypeFromString(blockJob)) <= 0) 
{
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown mirror job type '%s'"),
-   blockJob);
-VIR_FREE(blockJob);
+   _("unknown mirror job type '%s'"), blockJob);
 goto cleanup;
 }
-VIR_FREE(blockJob);
 } else {
 def->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
 }

-mirrorType = virXMLPropString(cur, "type");
-if (mirrorType) {
-def->mirror->type = virStorageTypeFromString(mirrorType);
-if (def->mirror->type <= 0) {
+if ((mirrorType = virXMLPropString(cur, "type"))) {
+if ((def->mirror->type = virStorageTypeFromString(mirrorType)) <= 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown mirror backing store "
- "type '%s'"), mirrorType);
+   _("unknown mirror backing store type '%s'"),
+   mirrorType);
 goto cleanup;
 }
-mirrorFormat = virXPathString("string(./mirror/format/@type)",
-  ctxt);
+
+mirrorFormat = virXPathString("string(./mirror/format/@type)", ctxt);
+
+if (!(mirrorNode = virXPathNode("./mirror/source", ctxt))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("mirror requires source element"));
+goto cleanup;
+}
+
+if (virDomainDiskSourceParse(mirrorNode, ctxt, def->mirror) < 0)
+goto cleanup;
 } else {
 /* For back-compat reasons, we handle a file name
  * encoded as attributes, even though we prefer
@@ -6807,44 +6810,28 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
 }
 mirrorFormat = virXMLPropString(cur, "format");
 }
+
 if (mirrorFormat) {
-def->mirror->format =
-virStorageFileFormatTypeFromString(mirrorFormat);
+def->mirror->format = virStorageFileFormatTypeFromString(mirrorFormat);
 if (def->mirror->format <= 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown mirror format value '%s'"),
-   mirrorFormat);
+   _("unknown mirror format value '%s'"), 
mirrorFormat);
 goto cleanup;
 }
 }
-if (mirrorType) {
-xmlNodePtr mirrorNode;

-if (!(mirrorNode = virXPathNode("./mirror/source", ctxt))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("mirror requires source element"));
-goto cleanup;
-}
-if (virDomainDiskSourceParse(mirrorNode, ctxt,
- def->mirror) < 0)
-goto cleanup;
-}
-ready = virXMLPropString(cur, "ready");
-if (ready) {
-if ((def->mirrorState =
- virDomainDiskMirrorStateTypeFromString(ready)) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("unknown mirror ready state %s"),
-   ready);
-VIR_FREE(ready);
-goto cleanup;
-}
-VIR_FREE(ready);
+if ((ready = virXMLPropString(cur, "ready")) &&
+(def->mirrorState = virDomainDiskMirrorStateTypeFromString(ready)) < 
0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("unknown mirror ready state %s"), ready);
+goto cleanup;
 }

 ret = 0;

  cleanup:
+VIR_FREE(ready);
+VIR_FREE(blockJob);
 VIR_FREE(mirrorType);
 VIR_FREE(mirrorFormat);
 return ret;
-- 
2.8.0

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

[libvirt] [PATCH 08/10] conf: disk: Split out parsing of disk mirror data

Changes are indentation and 'cleanup' label instead of 'error'.
---
 src/conf/domain_conf.c | 192 ++---
 1 file changed, 104 insertions(+), 88 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 64561df..802a6fe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6748,6 +6748,109 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 #undef PARSE_IOTUNE


+static int
+virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
+xmlNodePtr cur,
+xmlXPathContextPtr ctxt)
+{
+char *mirrorFormat = NULL;
+char *mirrorType = NULL;
+char *ready;
+char *blockJob;
+int ret = -1;
+
+if (VIR_ALLOC(def->mirror) < 0)
+goto cleanup;
+
+blockJob = virXMLPropString(cur, "job");
+if (blockJob) {
+def->mirrorJob = virDomainBlockJobTypeFromString(blockJob);
+if (def->mirrorJob <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown mirror job type '%s'"),
+   blockJob);
+VIR_FREE(blockJob);
+goto cleanup;
+}
+VIR_FREE(blockJob);
+} else {
+def->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
+}
+
+mirrorType = virXMLPropString(cur, "type");
+if (mirrorType) {
+def->mirror->type = virStorageTypeFromString(mirrorType);
+if (def->mirror->type <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown mirror backing store "
+ "type '%s'"), mirrorType);
+goto cleanup;
+}
+mirrorFormat = virXPathString("string(./mirror/format/@type)",
+  ctxt);
+} else {
+/* For back-compat reasons, we handle a file name
+ * encoded as attributes, even though we prefer
+ * modern output in the style of backingStore */
+def->mirror->type = VIR_STORAGE_TYPE_FILE;
+def->mirror->path = virXMLPropString(cur, "file");
+if (!def->mirror->path) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("mirror requires file name"));
+goto cleanup;
+}
+if (def->mirrorJob != VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("mirror without type only supported "
+ "by copy job"));
+goto cleanup;
+}
+mirrorFormat = virXMLPropString(cur, "format");
+}
+if (mirrorFormat) {
+def->mirror->format =
+virStorageFileFormatTypeFromString(mirrorFormat);
+if (def->mirror->format <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown mirror format value '%s'"),
+   mirrorFormat);
+goto cleanup;
+}
+}
+if (mirrorType) {
+xmlNodePtr mirrorNode;
+
+if (!(mirrorNode = virXPathNode("./mirror/source", ctxt))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("mirror requires source element"));
+goto cleanup;
+}
+if (virDomainDiskSourceParse(mirrorNode, ctxt,
+ def->mirror) < 0)
+goto cleanup;
+}
+ready = virXMLPropString(cur, "ready");
+if (ready) {
+if ((def->mirrorState =
+ virDomainDiskMirrorStateTypeFromString(ready)) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("unknown mirror ready state %s"),
+   ready);
+VIR_FREE(ready);
+goto cleanup;
+}
+VIR_FREE(ready);
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(mirrorType);
+VIR_FREE(mirrorFormat);
+return ret;
+}
+
+
 #define VENDOR_LEN  8
 #define PRODUCT_LEN 16

@@ -6799,8 +6902,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *vendor = NULL;
 char *product = NULL;
 char *discard = NULL;
-char *mirrorFormat = NULL;
-char *mirrorType = NULL;
 char *domain_name = NULL;
 int expected_secret_usage = -1;
 int auth_secret_usage = -1;
@@ -6934,91 +7035,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 } else if (!def->mirror &&
xmlStrEqual(cur->name, BAD_CAST "mirror") &&
!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
-char *ready;
-char *blockJob;
-
-if (VIR_ALLOC(def->mirror) < 0)
+if (virDomainDiskDefMirrorParse(def, cur, ctxt) < 0)
 goto error;
-
-blockJob = virXMLPropString(cur, "job");
-if (blockJob) {
-def->mirrorJob = virDomainBlockJobTypeFromString(blockJob);
-if (def->mirrorJob <= 0) {
-

[libvirt] [PATCH 10/10] conf: extract disk geometry parsing code

---
 src/conf/domain_conf.c | 75 +++---
 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e64471d..b5f2b91 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6838,6 +6838,51 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
 }


+static int
+virDomainDiskDefGeometryParse(virDomainDiskDefPtr def,
+  xmlNodePtr cur,
+  xmlXPathContextPtr ctxt)
+{
+char *trans;
+
+if (virXPathUInt("string(./geometry/@cyls)",
+ ctxt, >geometry.cylinders) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("invalid geometry settings (cyls)"));
+return -1;
+}
+
+if (virXPathUInt("string(./geometry/@heads)",
+ ctxt, >geometry.heads) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("invalid geometry settings (heads)"));
+return -1;
+}
+
+if (virXPathUInt("string(./geometry/@secs)",
+ ctxt, >geometry.sectors) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("invalid geometry settings (secs)"));
+return -1;
+}
+
+trans = virXMLPropString(cur, "trans");
+if (trans) {
+def->geometry.trans = virDomainDiskGeometryTransTypeFromString(trans);
+if (def->geometry.trans <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid translation value '%s'"),
+   trans);
+VIR_FREE(trans);
+return -1;
+}
+VIR_FREE(trans);
+}
+
+return 0;
+}
+
+
 #define VENDOR_LEN  8
 #define PRODUCT_LEN 16

@@ -6866,7 +6911,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *driverType = NULL;
 bool source = false;
 char *target = NULL;
-char *trans = NULL;
 char *bus = NULL;
 char *cachetag = NULL;
 char *error_policy = NULL;
@@ -6951,34 +6995,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur->name, BAD_CAST "backenddomain")) {
 domain_name = virXMLPropString(cur, "name");
 } else if (xmlStrEqual(cur->name, BAD_CAST "geometry")) {
-if (virXPathUInt("string(./geometry/@cyls)",
- ctxt, >geometry.cylinders) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("invalid geometry settings (cyls)"));
+if (virDomainDiskDefGeometryParse(def, cur, ctxt) < 0)
 goto error;
-}
-if (virXPathUInt("string(./geometry/@heads)",
- ctxt, >geometry.heads) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("invalid geometry settings (heads)"));
-goto error;
-}
-if (virXPathUInt("string(./geometry/@secs)",
- ctxt, >geometry.sectors) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("invalid geometry settings (secs)"));
-goto error;
-}
-trans = virXMLPropString(cur, "trans");
-if (trans) {
-def->geometry.trans = 
virDomainDiskGeometryTransTypeFromString(trans);
-if (def->geometry.trans <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("invalid translation value '%s'"),
-   trans);
-goto error;
-}
-}
 } else if (xmlStrEqual(cur->name, BAD_CAST "blockio")) {
 logical_block_size =
 virXMLPropString(cur, "logical_block_size");
@@ -7486,7 +7504,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 VIR_FREE(target);
 VIR_FREE(tray);
 VIR_FREE(removable);
-VIR_FREE(trans);
 VIR_FREE(device);
 virStorageAuthDefFree(authdef);
 VIR_FREE(driverType);
-- 
2.8.0

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

[libvirt] [PATCH 05/10] conf: disk: Remove error label from virDomainDiskDefIotuneParse

Since this function isn't doing any cleanup, the label is not necessary.
---
 src/conf/domain_conf.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c2ac8d6..ac464d3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6688,7 +6688,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 if (ret == -2) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("total throughput limit must be an integer"));
-goto error;
+return -1;
 } else if (ret < 0) {
 def->blkdeviotune.total_bytes_sec = 0;
 }
@@ -6699,7 +6699,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 if (ret == -2) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("read throughput limit must be an integer"));
-goto error;
+return -1;
 } else if (ret < 0) {
 def->blkdeviotune.read_bytes_sec = 0;
 }
@@ -6710,7 +6710,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 if (ret == -2) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("write throughput limit must be an integer"));
-goto error;
+return -1;
 } else if (ret < 0) {
 def->blkdeviotune.write_bytes_sec = 0;
 }
@@ -6721,7 +6721,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 if (ret == -2) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("total I/O operations limit must be an integer"));
-goto error;
+return -1;
 } else if (ret < 0) {
 def->blkdeviotune.total_iops_sec = 0;
 }
@@ -6732,7 +6732,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 if (ret == -2) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("read I/O operations limit must be an integer"));
-goto error;
+return -1;
 } else if (ret < 0) {
 def->blkdeviotune.read_iops_sec = 0;
 }
@@ -6743,7 +6743,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 if (ret == -2) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("write I/O operations limit must be an integer"));
-goto error;
+return -1;
 } else if (ret < 0) {
 def->blkdeviotune.write_iops_sec = 0;
 }
@@ -6798,7 +6798,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("total and read/write bytes_sec "
  "cannot be set at the same time"));
-goto error;
+return -1;
 }

 if ((def->blkdeviotune.total_iops_sec &&
@@ -6808,7 +6808,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("total and read/write iops_sec "
  "cannot be set at the same time"));
-goto error;
+return -1;
 }

 if ((def->blkdeviotune.total_bytes_sec_max &&
@@ -6818,7 +6818,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("total and read/write bytes_sec_max "
  "cannot be set at the same time"));
-goto error;
+return -1;
 }

 if ((def->blkdeviotune.total_iops_sec_max &&
@@ -6828,13 +6828,10 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("total and read/write iops_sec_max "
  "cannot be set at the same time"));
-goto error;
+return -1;
 }

 return 0;
-
- error:
-return ret;
 }


-- 
2.8.0

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

[libvirt] [PATCH 03/10] conf: disk: Remove one unnecessary level of indentation

Also simplify the code by switching to a for loop.
---
 src/conf/domain_conf.c | 737 -
 1 file changed, 368 insertions(+), 369 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aa96bac..f359e95 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6755,423 +6755,422 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 rawio = virXMLPropString(node, "rawio");
 sgio = virXMLPropString(node, "sgio");

-cur = node->children;
-while (cur != NULL) {
-if (cur->type == XML_ELEMENT_NODE) {
-if (!source && xmlStrEqual(cur->name, BAD_CAST "source")) {
-sourceNode = cur;
+for (cur = node->children; cur != NULL; cur = cur->next) {
+if (cur->type != XML_ELEMENT_NODE)
+continue;

-if (virDomainDiskSourceParse(cur, ctxt, def->src) < 0)
-goto error;
+if (!source && xmlStrEqual(cur->name, BAD_CAST "source")) {
+sourceNode = cur;

-source = true;
+if (virDomainDiskSourceParse(cur, ctxt, def->src) < 0)
+goto error;

-if (def->src->type == VIR_STORAGE_TYPE_NETWORK) {
-if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI)
-expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI;
-else if (def->src->protocol == 
VIR_STORAGE_NET_PROTOCOL_RBD)
-expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH;
-}
+source = true;

-startupPolicy = virXMLPropString(cur, "startupPolicy");
+if (def->src->type == VIR_STORAGE_TYPE_NETWORK) {
+if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI)
+expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI;
+else if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)
+expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH;
+}

-} else if (!target &&
-   xmlStrEqual(cur->name, BAD_CAST "target")) {
-target = virXMLPropString(cur, "dev");
-bus = virXMLPropString(cur, "bus");
-tray = virXMLPropString(cur, "tray");
-removable = virXMLPropString(cur, "removable");
-
-/* HACK: Work around for compat with Xen
- * driver in previous libvirt releases */
-if (target &&
-STRPREFIX(target, "ioemu:"))
-memmove(target, target+6, strlen(target)-5);
-} else if (!domain_name &&
-   xmlStrEqual(cur->name, BAD_CAST "backenddomain")) {
-domain_name = virXMLPropString(cur, "name");
-} else if (xmlStrEqual(cur->name, BAD_CAST "geometry")) {
-if (virXPathUInt("string(./geometry/@cyls)",
- ctxt, >geometry.cylinders) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("invalid geometry settings (cyls)"));
-goto error;
-}
-if (virXPathUInt("string(./geometry/@heads)",
- ctxt, >geometry.heads) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("invalid geometry settings (heads)"));
-goto error;
-}
-if (virXPathUInt("string(./geometry/@secs)",
- ctxt, >geometry.sectors) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("invalid geometry settings (secs)"));
-goto error;
-}
-trans = virXMLPropString(cur, "trans");
-if (trans) {
-def->geometry.trans = 
virDomainDiskGeometryTransTypeFromString(trans);
-if (def->geometry.trans <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("invalid translation value '%s'"),
-   trans);
-goto error;
-}
-}
-} else if (xmlStrEqual(cur->name, BAD_CAST "blockio")) {
-logical_block_size =
-virXMLPropString(cur, "logical_block_size");
-if (logical_block_size &&
-virStrToLong_ui(logical_block_size, NULL, 0,
->blockio.logical_block_size) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("invalid logical block size '%s'"),
-   logical_block_size);
-goto error;
-}
-

[libvirt] [PATCH 00/10] conf: disk xml parser refactors