[libvirt] [PATCH] qemu: fix migration flags undefinesource cannot work

[Luyao Huang]

In commit f41be296, we moved vm->persistent check into
qemuDomainRemoveInactive, but we didn't change the vm->persistent
before call qemuDomainRemoveInactive in some place before and just
call it to remove the inactive vm.

Signed-off-by: Luyao Huang 
---
 src/qemu/qemu_migration.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b53491a..2abf2ee 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3912,8 +3912,10 @@ qemuMigrationConfirm(virConnectPtr conn,
 
 qemuMigrationJobFinish(driver, vm);
 if (!virDomainObjIsActive(vm)) {
-if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
+if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
 virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
+vm->persistent = 0;
+}
 qemuDomainRemoveInactive(driver, vm);
 }
 
@@ -5405,8 +5407,10 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,
 
 qemuMigrationJobFinish(driver, vm);
 if (!virDomainObjIsActive(vm) && ret == 0) {
-if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
+if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
 virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
+vm->persistent = 0;
+}
 qemuDomainRemoveInactive(driver, vm);
 }
 
-- 
1.8.3.1

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

[libvirt] [PATCH] libvirt-domain: fix no error report when p2p migrate fail

[Luyao Huang]

After commit a26669d7, we only jump to error when
virDomainMigrateUnmanagedParams return a value less than -1.
this will make the migrate result always be success even we
meet some problem.

Signed-off-by: Luyao Huang 
---
 src/libvirt-domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 6e1aacd..14aeb09 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -4478,7 +4478,7 @@ virDomainMigrateToURI3(virDomainPtr domain,
 dconnuri = NULL;
 
 if (virDomainMigrateUnmanagedParams(domain, dconnuri,
-params, nparams, flags) < -1)
+params, nparams, flags) < 0)
 goto error;
 
 return 0;
-- 
1.8.3.1

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

[libvirt] [PATCH] libvirt-domain: fix the error reporting when use the localhost as target uri

[Luyao Huang]

Remove the extra %s in error message when call virReportInvalidArg().

Signed-off-by: Luyao Huang 
---
 src/libvirt-domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 14aeb09..de7eb04 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3303,7 +3303,7 @@ virDomainMigrateCheckNotLocal(const char *dconnuri)
 goto cleanup;
 if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
 virReportInvalidArg(dconnuri, "%s",
-_("Attempt to migrate guest to the same host %s"));
+_("Attempt to migrate guest to the same host"));
 goto cleanup;
 }
 
-- 
1.8.3.1

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

Re: [libvirt] [PATCH v2 0/5] qemu: hostdev: Unify naming

[Andrea Bolognani]

On Mon, 2015-10-26 at 18:15 -0400, John Ferlan wrote:
> >   hostdev: Rename virHostdevUpdateDomainActiveDevices()
> >   qemu: hostdev: Unify naming for qemuHostdevPrepare*Devices()
> >   qemu: hostdev: Unify naming for qemuHostdevReAttach*Devices()
> >   qemu: hostdev: Unify naming for qemuHostdevUpdateActive*Devices()
> >   qemu: hostdev: Introduce qemuHostdevUpdateActiveDomainDevices()
> > 
> >  src/libvirt_private.syms |   2 +-
> >  src/libxl/libxl_driver.c |   2 +-
> >  src/qemu/qemu_hostdev.c  | 108 +++
> > 
> >  src/qemu/qemu_hostdev.h  |  69 +++---
> >  src/qemu/qemu_hotplug.c  |  20 -
> >  src/qemu/qemu_process.c  |  14 ++
> >  src/util/virhostdev.c|   2 +-
> >  src/util/virhostdev.h|   2 +-
> >  8 files changed, 117 insertions(+), 102 deletions(-)
> > 
> 
> ACK series -

Pushed, thanks :)

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH] domain-conf: reorder usb controllers so the master is first

[Ján Tomko]

On Mon, Oct 26, 2015 at 05:28:21PM +0100, Pavel Hrdina wrote:
> USB controllers can share the same 'index' which indicates, that there
> is some sort of master-companion relationship.  Reorder the controllers
> in XML in to place the master controller before its companions.  This is
> required by QEMU to not fail with error message:
> 
> error: internal error: process exited while connecting to monitor:
> 2015-10-26T16:25:17.630265Z qemu-system-x86_64:
> -device 
> ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x6:
> USB bus 'usb.0' not found
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1166452
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/conf/domain_conf.c | 33 ++---
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 

Makes sense, but the patch would look better with -b if the
non-functional changes were separate.
A test case would be nice too.

Jan


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

[libvirt] [PATCH v2 1/2] domain-conf: reorder usb controllers so the master is first

[Pavel Hrdina]

USB controllers can share the same 'index' which indicates, that there
is some sort of master-companion relationship.  Reorder the controllers
in XML in to place the master controller before its companions.  This is
required by QEMU to not fail with error message:

error: internal error: process exited while connecting to monitor:
2015-10-26T16:25:17.630265Z qemu-system-x86_64:
-device 
ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x6:
USB bus 'usb.0' not found

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

Signed-off-by: Pavel Hrdina 
---
 src/conf/domain_conf.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0c559d2..3f22de2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13397,6 +13397,7 @@ void 
virDomainControllerInsertPreAlloced(virDomainDefPtr def,
 int idx;
 /* Tenatively plan to insert controller at the end. */
 int insertAt = -1;
+virDomainControllerDefPtr current = NULL;
 
 /* Then work backwards looking for controllers of
  * the same type. If we find a controller with a
@@ -13404,19 +13405,29 @@ void 
virDomainControllerInsertPreAlloced(virDomainDefPtr def,
  * that position
  */
 for (idx = (def->ncontrollers - 1); idx >= 0; idx--) {
+current = def->controllers[idx];
+if (current->type == controller->type) {
+if (current->idx > controller->idx) {
 /* If bus matches and current controller is after
- * new controller, then new controller should go here */
-if (def->controllers[idx]->type == controller->type &&
-def->controllers[idx]->idx > controller->idx) {
+ * new controller, then new controller should go here
+ * */
 insertAt = idx;
-} else if (def->controllers[idx]->type == controller->type &&
-   insertAt == -1) {
+} else if (controller->info.mastertype == 
VIR_DOMAIN_CONTROLLER_MASTER_NONE &&
+   current->info.mastertype != 
VIR_DOMAIN_CONTROLLER_MASTER_NONE &&
+   current->idx == controller->idx) {
+/* If bus matches and index matches and new controller is
+ * master and current isn't a master, then new controller
+ * should go here to be placed before its companion
+ */
+insertAt = idx;
+} else if (insertAt == -1) {
 /* Last controller with match bus is before the
  * new controller, then put new controller just after
  */
 insertAt = idx + 1;
 }
 }
+}
 
 /* VIR_INSERT_ELEMENT_INPLACE will never return an error here. */
 ignore_value(VIR_INSERT_ELEMENT_INPLACE(def->controllers, insertAt,
-- 
2.6.2

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

[libvirt] [PATCH v2 0/2] reorder usb controllers correctly

[Pavel Hrdina]

new in v2:
 - used git format-patch -b
 - added a test case

Pavel Hrdina (2):
  domain-conf: reorder usb controllers so the master is first
  test: add test case for usb controller order

 src/conf/domain_conf.c | 21 +
 .../qemuxml2argv-controller-usb-order.xml  | 32 
 .../qemuxml2xmlout-controller-usb-order.xml| 34 ++
 tests/qemuxml2xmltest.c|  1 +
 4 files changed, 83 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml

-- 
2.6.2

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

Re: [libvirt] [PATCH v2 0/2] reorder usb controllers correctly

[Peter Krempa]

On Tue, Oct 27, 2015 at 10:37:27 +0100, Pavel Hrdina wrote:
> new in v2:
>  - used git format-patch -b

I think Jan meant that it was hard to review with that option due to the
fact that it contains both whitespace and functional changes, not that
you should actually use it to send patches. They can't really be applied
that way.

Peter


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

Re: [libvirt] [PATCH 0/4] HACKING: Improve handling

[Ján Tomko]

On Mon, Oct 26, 2015 at 05:05:49PM +0100, Andrea Bolognani wrote:
> On Mon, 2015-10-26 at 16:25 +0100, Michal Privoznik wrote:
> > On 26.10.2015 16:03, Andrea Bolognani wrote:
> > > Generate the HACKING file at dist time like we're already doing for
> > > NEWS, AUTHORS and ChangeLog.
> > > 
> > > The file shouldn't be tracked by git as it's not a source file but
> > > rather a generated one; because of this, it should also be
> > > generated
> > > in $(builddir) as opposed to $(top_srcdir).
> > > 
> > > Cheers.
> > > 
> > > 
> > > Andrea Bolognani (4):
> > >   HACKING: Generate inside the build directory
> > >   HACKING: Remove generated copy
> > >   HACKING: Add to ignore file
> > >   HACKING: Include in EXTRA_DIST
> > > 
> > >  .gitignore  |1 +
> > >  HACKING | 1008 ---
> > > 
> > >  Makefile.am |3 +-
> > >  cfg.mk  |3 +-
> > >  4 files changed, 4 insertions(+), 1011 deletions(-)
> > >  delete mode 100644 HACKING
> > > 
> > 
> > Frankly, I'd like to keep HACKING in the git. I know it's a generated
> > file, but the reasoning should be that a new contributor, who is
> > about
> > to make his first contribution will clone the repo and the first
> > thing
> > they should search for is Readme and HACKING files. I know that
> > dealing
> > with the file which is then again generated from another file,
> > keeping
> > them both in git may be counterintuitive, but I'd like to keep the
> > file
> > there.
> 
> We also have README-hacking which contains information that
> is arguably more vital to a first-time contributor, eg. how
> to bootstrap the development environment.
> 

Successfully building libvirt (which should generate HACKING) is a
prerequisite for making any changes to it anyway (except for
translations).

A large part of 'HACKING' is currently dealing with whitespace
and brace rules, which are enforced by syntax-check. Moving them to a
separate file (CodingStyle) or deleting them completely and pointing
new contributors to running 'make syntax-check' with cppi installed
would reduce the length of the file, thus making it more likely to be read
completely.

> Other possible ways to approach the problem:
> 
>   * include a note in README-hacking telling the user to
> run 'make HACKING' to build the contributor guidelines
> 

I vote for this option. Let's keep the essential information in
README-hacking, and the information needed for larger patches
and second-time contributors in HACKING.

Jan


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

Re: [libvirt] [PATCH] locking: Add io_timeout to sanlock

[Martin Kletzander]

On Fri, Oct 23, 2015 at 01:37:54PM +0200, Michal Privoznik wrote:

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

So, if domain loses access to storage, sanlock tries to kill it
after some timeout. So far, the default is 80 seconds. But for
some scenarios this might not be enough. We should allow users to
adjust the timeout according to their needs.



Shouldn't we check for whether the current sanlock version supports
that?  Or require new enough version in configure/libvirt.spec?  I
understand the _timeout option was added later then the previous one.

Otherwise looks good, if you convince me that there is no need for the
check then it can go in as it is.

Martin


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

[libvirt] [PATCH v2 2/2] test: add test case for usb controller order

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 .../qemuxml2argv-controller-usb-order.xml  | 32 
 .../qemuxml2xmlout-controller-usb-order.xml| 34 ++
 tests/qemuxml2xmltest.c|  1 +
 3 files changed, 67 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml
new file mode 100644
index 000..68ebbf2
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml
@@ -0,0 +1,32 @@
+
+  rhel7
+  c9b867fb-7274-4a22-8884-0867d05b38cf
+  2097152
+  2097152
+  2
+  
+hvm
+
+  
+  destroy
+  restart
+  restart
+  
+
+
+  
+  
+/usr/bin/qemu-system-x86_64
+
+  
+  
+
+
+  
+
+
+  
+
+  
+
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml
new file mode 100644
index 000..a3c22e8
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml
@@ -0,0 +1,34 @@
+
+  rhel7
+  c9b867fb-7274-4a22-8884-0867d05b38cf
+  2097152
+  2097152
+  2
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  restart
+  
+
+
+  
+  
+/usr/bin/qemu-system-x86_64
+
+  
+
+
+  
+  
+
+
+  
+
+
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 5a9c67d..5e6fc9a 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -515,6 +515,7 @@ mymain(void)
 DO_TEST_DIFFERENT("usb-redir-filter");
 DO_TEST_DIFFERENT("usb-redir-filter-version");
 DO_TEST("blkdeviotune");
+DO_TEST_DIFFERENT("controller-usb-order");
 
 DO_TEST_FULL("seclabel-dynamic-baselabel", false, WHEN_INACTIVE);
 DO_TEST_FULL("seclabel-dynamic-override", false, WHEN_INACTIVE);
-- 
2.6.2

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

Re: [libvirt] [PATCH] qemu: fix migration flags undefinesource cannot work

[Martin Kletzander]

On Tue, Oct 27, 2015 at 04:53:59PM +0800, Luyao Huang wrote:

In commit f41be296, we moved vm->persistent check into
qemuDomainRemoveInactive, but we didn't change the vm->persistent
before call qemuDomainRemoveInactive in some place before and just
call it to remove the inactive vm.

Signed-off-by: Luyao Huang 
---
src/qemu/qemu_migration.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)



ACK to all three, pushed now.


diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b53491a..2abf2ee 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3912,8 +3912,10 @@ qemuMigrationConfirm(virConnectPtr conn,

qemuMigrationJobFinish(driver, vm);
if (!virDomainObjIsActive(vm)) {
-if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
+if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
+vm->persistent = 0;
+}
qemuDomainRemoveInactive(driver, vm);
}

@@ -5405,8 +5407,10 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,

qemuMigrationJobFinish(driver, vm);
if (!virDomainObjIsActive(vm) && ret == 0) {
-if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
+if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
+vm->persistent = 0;
+}
qemuDomainRemoveInactive(driver, vm);
}

--
1.8.3.1

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


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

Re: [libvirt] [PATCH] util: implement virProcessGetStartTime on GNU/kFreeBSD

[Martin Kletzander]

On Thu, Oct 15, 2015 at 01:50:42PM +0200, Pino Toscano wrote:

Use the virProcessGetStartTime implementation also when only the kernel
is FreeBSD, such as on GNU/kFreeBSD.
---
src/util/virprocess.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index e6b78ef..43118f8 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -36,11 +36,11 @@
# include 
#endif

-#if defined(__FreeBSD__) || HAVE_BSD_CPU_AFFINITY
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
HAVE_BSD_CPU_AFFINITY


I'm guessing __FreeBSD_kernel__ is not defined when compiling on pure
FreeBSD, so we cannot use just __FreeBSD_kernel__, right?

Weak ACK -- I have nowhere to try it and I'm lazy to install such
machine just for trying out this patch, but I can clearly see it won't
break anything that works right now, so...

Also, if this works, then I believe there will be way more functions
that could use this reasoning to make libvirt more Whatever/kFreeBSD
friendly.  Would you care to check some of those, like
virSecurityDACGetProcessLabelInternal() etc.?

I'm not pushing it if someone else has a chance to try it or review it
since I'm not \kFreeBSD experienced.


# include 
#endif

-#ifdef  __FreeBSD__
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
# include 
# include 
#endif
@@ -937,7 +937,7 @@ int virProcessGetStartTime(pid_t pid,
VIR_FREE(buf);
return ret;
}
-#elif defined(__FreeBSD__)
+#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
int virProcessGetStartTime(pid_t pid,
   unsigned long long *timestamp)
{
--
2.1.0

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


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

Re: [libvirt] [PATCH] conf: Add serial target type to ABI stability check

[Martin Kletzander]

On Wed, Oct 21, 2015 at 03:14:03PM +0800, Luyao Huang wrote:

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

There is no ABI check for serial target type attribute, just
add it.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c | 8 
1 file changed, 8 insertions(+)



ACK && Pushed


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02cc8ad..a31bc05 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17225,6 +17225,14 @@ static bool
virDomainSerialDefCheckABIStability(virDomainChrDefPtr src,
virDomainChrDefPtr dst)
{
+if (src->targetType != dst->targetType) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target serial type %s does not match source %s"),
+   virDomainChrSerialTargetTypeToString(dst->targetType),
+   virDomainChrSerialTargetTypeToString(src->targetType));
+return false;
+}
+
if (src->target.port != dst->target.port) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("Target serial port %d does not match source %d"),
--
1.8.3.1

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


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

Re: [libvirt] [PATCH] util: implement virProcessGetStartTime on GNU/kFreeBSD

[Andrea Bolognani]

On Tue, 2015-10-27 at 14:54 +0300, Roman Bogorodskiy wrote:
> > Weak ACK -- I have nowhere to try it and I'm lazy to install such
> > machine just for trying out this patch, but I can clearly see it
> > won't
> > break anything that works right now, so...
> 
> FWIW, it also looks fine to me (though I do not have GNU\kFreeBSD
> boxes
> as well, only FreeBSD ones). I can build-test on FreeBSD just to be
> on a
> safe side, but it doesn't seem like it could break something indeed.

I'd like to point out that FreeBSD builds are now performed
regularly, so any breakage affecting FreeBDS should be detected
pretty quickly.

https://ci.centos.org/view/libvirt-project/job/libvirt-freebsd/

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH] util: implement virProcessGetStartTime on GNU/kFreeBSD

[Roman Bogorodskiy]

  Martin Kletzander wrote:

> On Thu, Oct 15, 2015 at 01:50:42PM +0200, Pino Toscano wrote:
> >Use the virProcessGetStartTime implementation also when only the kernel
> >is FreeBSD, such as on GNU/kFreeBSD.
> >---
> > src/util/virprocess.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> >index e6b78ef..43118f8 100644
> >--- a/src/util/virprocess.c
> >+++ b/src/util/virprocess.c
> >@@ -36,11 +36,11 @@
> > # include 
> > #endif
> >
> >-#if defined(__FreeBSD__) || HAVE_BSD_CPU_AFFINITY
> >+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
> >HAVE_BSD_CPU_AFFINITY
> 
> I'm guessing __FreeBSD_kernel__ is not defined when compiling on pure
> FreeBSD, so we cannot use just __FreeBSD_kernel__, right?

That's true.

> Weak ACK -- I have nowhere to try it and I'm lazy to install such
> machine just for trying out this patch, but I can clearly see it won't
> break anything that works right now, so...

FWIW, it also looks fine to me (though I do not have GNU\kFreeBSD boxes
as well, only FreeBSD ones). I can build-test on FreeBSD just to be on a
safe side, but it doesn't seem like it could break something indeed.

Roman Bogorodskiy

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

[libvirt] [PATCH 2/5] qemu: Rename cleanup label in qemuProcessStart

[Jiri Denemark]

Current "cleanup" label is only used in error path, thus it should
rather be called "error".

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 178 
 1 file changed, 89 insertions(+), 89 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8aa9bb9..13433f8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4422,14 +4422,14 @@ int qemuProcessStart(virConnectPtr conn,
 }
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
-goto cleanup;
+goto error;
 
 /* Some things, paths, ... are generated here and we want them to persist.
  * Fill them in prior to setting the domain def as transient. */
 VIR_DEBUG("Generating paths");
 
 if (qemuPrepareNVRAM(cfg, vm, !!migrateFrom) < 0)
-goto cleanup;
+goto error;
 
 /* Do this upfront, so any part of the startup process can add
  * runtime state to vm->def that won't be persisted. This let's us
@@ -4437,7 +4437,7 @@ int qemuProcessStart(virConnectPtr conn,
  */
 VIR_DEBUG("Setting current domain def as transient");
 if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0)
-goto cleanup;
+goto error;
 
 vm->def->id = qemuDriverAllocateID(driver);
 qemuDomainSetFakeReboot(driver, vm, false);
@@ -4460,7 +4460,7 @@ int qemuProcessStart(virConnectPtr conn,
  * If the script raised an error abort the launch
  */
 if (hookret < 0)
-goto cleanup;
+goto error;
 }
 
 VIR_DEBUG("Determining emulator version");
@@ -4468,7 +4468,7 @@ int qemuProcessStart(virConnectPtr conn,
 if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
   vm->def->emulator,
   vm->def->os.machine)))
-goto cleanup;
+goto error;
 
 /* network devices must be "prepared" before hostdevs, because
  * setting up a network device might create a new hostdev that
@@ -4476,7 +4476,7 @@ int qemuProcessStart(virConnectPtr conn,
  */
 VIR_DEBUG("Preparing network devices");
 if (qemuNetworkPrepareDevices(vm->def) < 0)
-   goto cleanup;
+   goto error;
 
 /* Must be run before security labelling */
 VIR_DEBUG("Preparing host devices");
@@ -4486,25 +4486,25 @@ int qemuProcessStart(virConnectPtr conn,
 hostdev_flags |= VIR_HOSTDEV_COLD_BOOT;
 if (qemuHostdevPrepareDomainDevices(driver, vm->def, priv->qemuCaps,
 hostdev_flags) < 0)
-goto cleanup;
+goto error;
 
 VIR_DEBUG("Preparing chr devices");
 if (virDomainChrDefForeach(vm->def,
true,
qemuProcessPrepareChardevDevice,
NULL) < 0)
-goto cleanup;
+goto error;
 
 VIR_DEBUG("Checking domain and device security labels");
 if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) < 0)
-goto cleanup;
+goto error;
 
 /* If you are using a SecurityDriver with dynamic labelling,
then generate a security label for isolation */
 VIR_DEBUG("Generating domain security label (if required)");
 if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) {
 virDomainAuditSecurityLabel(vm, false);
-goto cleanup;
+goto error;
 }
 virDomainAuditSecurityLabel(vm, true);
 
@@ -4513,14 +4513,14 @@ int qemuProcessStart(virConnectPtr conn,
 char *hugepagePath = qemuGetHugepagePath(>hugetlbfs[i]);
 
 if (!hugepagePath)
-goto cleanup;
+goto error;
 
 if (virSecurityManagerSetHugepages(driver->securityManager,
vm->def, hugepagePath) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to set huge path in security 
driver"));
 VIR_FREE(hugepagePath);
-goto cleanup;
+goto error;
 }
 VIR_FREE(hugepagePath);
 }
@@ -4538,7 +4538,7 @@ int qemuProcessStart(virConnectPtr conn,
 if (virPortAllocatorSetUsed(driver->remotePorts,
 graphics->data.vnc.port,
 true) < 0) {
-goto cleanup;
+goto error;
 }
 
 graphics->data.vnc.portReserved = true;
@@ -4550,7 +4550,7 @@ int qemuProcessStart(virConnectPtr conn,
 if (virPortAllocatorSetUsed(driver->remotePorts,
 graphics->data.spice.port,
 true) < 0)
-goto cleanup;
+goto 

[libvirt] [PATCH 1/5] qemu: Use correct type when calling qemuPrepareNVRAM

[Jiri Denemark]

qemuProcessStart was passing char * migrateFrom as the third argument to
qemuPrepareNVRAM. We should explicitly convert the pointer to bool which
is what the function expects.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f744419..8aa9bb9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4428,7 +4428,7 @@ int qemuProcessStart(virConnectPtr conn,
  * Fill them in prior to setting the domain def as transient. */
 VIR_DEBUG("Generating paths");
 
-if (qemuPrepareNVRAM(cfg, vm, migrateFrom) < 0)
+if (qemuPrepareNVRAM(cfg, vm, !!migrateFrom) < 0)
 goto cleanup;
 
 /* Do this upfront, so any part of the startup process can add
-- 
2.6.2

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

[libvirt] [PATCH v2 0/3] improve virsh attach-interface

[Pavel Hrdina]

new in v2:
 - removed netdev name as an option for --source parameter
 - removed --driver parameter, the default is vfio for new qemu and that's good
 enough

Pavel Hrdina (3):
  virsh-nodedev: makes struct and functions for NodeDevice list
available
  virsh-domain: update attach-interface to support type=hostdev
  virsh.pod: update and improve a attach-interface section

 tools/virsh-domain.c  | 34 +++--
 tools/virsh-nodedev.c | 16 --
 tools/virsh-nodedev.h | 11 +++
 tools/virsh.pod   | 82 +++
 4 files changed, 98 insertions(+), 45 deletions(-)

-- 
2.6.2

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

[libvirt] [PATCH v2 1/3] virsh-nodedev: makes struct and functions for NodeDevice list available

[Pavel Hrdina]

Next patch will use those function to collect NodeDevice list and find a
specific device.  Make functions virshNodeDeviceListCollect() and
virshNodeDeviceListFree() together with struct virshNodeDeviceList
available to reuse existing code.

Signed-off-by: Pavel Hrdina 
---
 tools/virsh-nodedev.c | 16 +---
 tools/virsh-nodedev.h | 11 +++
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index cc359e2..26f2c7b 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -194,13 +194,7 @@ virshNodeDeviceSorter(const void *a, const void *b)
  virNodeDeviceGetName(*nb));
 }
 
-struct virshNodeDeviceList {
-virNodeDevicePtr *devices;
-size_t ndevices;
-};
-typedef struct virshNodeDeviceList *virshNodeDeviceListPtr;
-
-static void
+void
 virshNodeDeviceListFree(virshNodeDeviceListPtr list)
 {
 size_t i;
@@ -215,11 +209,11 @@ virshNodeDeviceListFree(virshNodeDeviceListPtr list)
 VIR_FREE(list);
 }
 
-static virshNodeDeviceListPtr
+virshNodeDeviceListPtr
 virshNodeDeviceListCollect(vshControl *ctl,
- char **capnames,
- int ncapnames,
- unsigned int flags)
+   char **capnames,
+   int ncapnames,
+   unsigned int flags)
 {
 virshNodeDeviceListPtr list = vshMalloc(ctl, sizeof(*list));
 size_t i;
diff --git a/tools/virsh-nodedev.h b/tools/virsh-nodedev.h
index c64f7df..1d2337b 100644
--- a/tools/virsh-nodedev.h
+++ b/tools/virsh-nodedev.h
@@ -30,4 +30,15 @@
 
 extern const vshCmdDef nodedevCmds[];
 
+struct virshNodeDeviceList {
+virNodeDevicePtr *devices;
+size_t ndevices;
+};
+typedef struct virshNodeDeviceList *virshNodeDeviceListPtr;
+
+virshNodeDeviceListPtr virshNodeDeviceListCollect(vshControl *ctl,
+  char **capnames,
+  int ncapnames,
+  unsigned int flags);
+void virshNodeDeviceListFree(virshNodeDeviceListPtr list);
 #endif /* VIRSH_NODEDEV_H */
-- 
2.6.2

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

[libvirt] [PATCH v2 3/3] virsh.pod: update and improve a attach-interface section

[Pavel Hrdina]

Rewrite the attach-interface section in man page to be more readable and
add the new hostdev functionality.

Signed-off-by: Pavel Hrdina 
---
 tools/virsh.pod | 82 +++--
 1 file changed, 50 insertions(+), 32 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0212e7a..843c293 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode 
shareable>.
 [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
 [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]
 [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>]
-[I<--print-xml>]
-
-Attach a new network interface to the domain.  I can be
-I to indicate connection via a libvirt virtual network, or
-I to indicate connection via a bridge device on the host, or
-I to indicate connection directly to one of the host's network
-interfaces or bridges.  I indicates the source of the
-connection (the name of a network, or of a bridge device, or the
-host's network interfaces or bridges).  I is used to specify
-the tap/macvtap device to be used to connect the domain to the
-source. Names starting with 'vnet' are considered as auto-generated
-and are blanked out/regenerated each time the interface is attached.
-I specifies the MAC address of the network interface; if a MAC
+[I<--managed>] [I<--print-xml>]
+
+Attach a new network interface to the domain.
+
+B<--type> can be one of the: I to indicate connection via a libvirt
+virtual network, I to indicate connection via a bridge device
+on the host, I to indicate connection directly to one of the host's
+network interfaces or bridges, I to indicate connection using a
+passthrough of PCI device on the host.
+
+B<--source> indicates the source of the connection.  The source depends
+on the type of the interface: I name of the virtual network,
+I the name of the bridge device, I the name of the host's
+interface or bridge, I the PCI address of the host's interface
+formatted as domain:bus:slot.function.
+
+B<--target> is used to specify the tap/macvtap device to be used to
+connect the domain to the source.  Names starting with 'vnet' are
+considered as auto-generated and are blanked out/regenerated each
+time the interface is attached.
+
+B<--mac> specifies the MAC address of the network interface; if a MAC
 address is not given, a new address will be automatically generated
 (and stored in the persistent configuration if "--config" is given on
-the commandline).  

[libvirt] [PATCH 5/5] qemu: Fix memory leak in qemuProcessStart

[Jiri Denemark]

nodeset should be freed in both success and failure paths.

While tmppath is freed immediately after it's consumed, moving it from
error to cleanup label is a bit more consistent and robust.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 92eab3c..524072c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5104,14 +5104,14 @@ int qemuProcessStart(virConnectPtr conn,
 virObjectUnref(cfg);
 virObjectUnref(caps);
 VIR_FREE(nicindexes);
+VIR_FREE(nodeset);
+VIR_FREE(tmppath);
 return ret;
 
  error:
 /* We jump here if we failed to start the VM for any reason, or
  * if we failed to initialize the now running VM. kill it off and
  * pretend we never started it */
-VIR_FREE(tmppath);
-VIR_FREE(nodeset);
 if (priv->mon)
 qemuMonitorSetDomainLog(priv->mon, -1);
 qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags);
-- 
2.6.2

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

[libvirt] [PATCH 4/5] qemu: Introduce cleanup label in qemuProcessStart

[Jiri Denemark]

Remove code duplication by moving common cleanup code in a dedicated
label.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ba6d292..92eab3c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4365,6 +4365,7 @@ int qemuProcessStart(virConnectPtr conn,
  virNetDevVPortProfileOp vmop,
  unsigned int flags)
 {
+int ret = -1;
 int rv;
 off_t pos = -1;
 char ebuf[1024];
@@ -5095,13 +5096,15 @@ int qemuProcessStart(virConnectPtr conn,
 if (!migrateFrom)
 qemuMonitorSetDomainLog(priv->mon, -1);
 
+ret = 0;
+
+ cleanup:
 virCommandFree(cmd);
 VIR_FORCE_CLOSE(logfile);
 virObjectUnref(cfg);
 virObjectUnref(caps);
 VIR_FREE(nicindexes);
-
-return 0;
+return ret;
 
  error:
 /* We jump here if we failed to start the VM for any reason, or
@@ -5109,16 +5112,10 @@ int qemuProcessStart(virConnectPtr conn,
  * pretend we never started it */
 VIR_FREE(tmppath);
 VIR_FREE(nodeset);
-virCommandFree(cmd);
-VIR_FORCE_CLOSE(logfile);
 if (priv->mon)
 qemuMonitorSetDomainLog(priv->mon, -1);
 qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags);
-virObjectUnref(cfg);
-virObjectUnref(caps);
-VIR_FREE(nicindexes);
-
-return -1;
+goto cleanup;
 
  exit_monitor:
 ignore_value(qemuDomainObjExitMonitor(driver, vm));
-- 
2.6.2

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

[libvirt] [PATCH] Remove new lines from debug messages

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/nwfilter/nwfilter_gentech_driver.c |  2 +-
 src/nwfilter/nwfilter_learnipaddr.c| 10 +-
 src/rpc/virnetsocket.c |  2 +-
 src/util/virfile.c |  2 +-
 src/util/virhash.c |  2 +-
 src/util/virnetdevmacvlan.c| 28 +-
 src/util/virprocess.c  |  2 +-
 src/xen/xend_internal.c|  2 +-
 tests/virhostdevtest.c | 36 +-
 tests/virnetsockettest.c   |  2 +-
 tests/virtimetest.c|  2 +-
 11 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c 
b/src/nwfilter/nwfilter_gentech_driver.c
index 701f8d8..5a4cff1 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -540,7 +540,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
 if (rc)
 break;
 } else if (inc) {
-VIR_DEBUG("Following filter %s\n", inc->filterref);
+VIR_DEBUG("Following filter %s", inc->filterref);
 obj = virNWFilterObjFindByName(>nwfilters, inc->filterref);
 if (obj) {
 
diff --git a/src/nwfilter/nwfilter_learnipaddr.c 
b/src/nwfilter/nwfilter_learnipaddr.c
index 5b55055..1adbadb 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -413,7 +413,7 @@ learnIPAddressThread(void *arg)
 handle = pcap_open_live(listen_if, BUFSIZ, 0, PKT_TIMEOUT_MS, errbuf);
 
 if (handle == NULL) {
-VIR_DEBUG("Couldn't open device %s: %s\n", listen_if, errbuf);
+VIR_DEBUG("Couldn't open device %s: %s", listen_if, errbuf);
 req->status = ENODEV;
 goto done;
 }
@@ -448,13 +448,13 @@ learnIPAddressThread(void *arg)
 filter = virBufferContentAndReset();
 
 if (pcap_compile(handle, , filter, 1, 0) != 0) {
-VIR_DEBUG("Couldn't compile filter '%s'.\n", filter);
+VIR_DEBUG("Couldn't compile filter '%s'", filter);
 req->status = EINVAL;
 goto done;
 }
 
 if (pcap_setfilter(handle, ) != 0) {
-VIR_DEBUG("Couldn't set filter '%s'.\n", filter);
+VIR_DEBUG("Couldn't set filter '%s'", filter);
 req->status = EINVAL;
 pcap_freecode();
 goto done;
@@ -626,7 +626,7 @@ learnIPAddressThread(void *arg)
req->filtername,
req->filterparams);
 VIR_DEBUG("Result from applying firewall rules on "
-  "%s with IP addr %s : %d\n", req->ifname, inetaddr, ret);
+  "%s with IP addr %s : %d", req->ifname, inetaddr, ret);
 }
 } else {
 if (showError)
@@ -638,7 +638,7 @@ learnIPAddressThread(void *arg)
 techdriver->applyDropAllRules(req->ifname);
 }
 
-VIR_DEBUG("pcap thread terminating for interface %s\n", req->ifname);
+VIR_DEBUG("pcap thread terminating for interface %s", req->ifname);
 
 virNWFilterUnlockIface(req->ifname);
 
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 5e5f1ab..526d291 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -196,7 +196,7 @@ int virNetSocketCheckProtocols(bool *hasIPv4,
 
 freeaddrinfo(ai);
 
-VIR_DEBUG("Protocols: v4 %d v6 %d\n", *hasIPv4, *hasIPv6);
+VIR_DEBUG("Protocols: v4 %d v6 %d", *hasIPv4, *hasIPv6);
 
 ret = 0;
  cleanup:
diff --git a/src/util/virfile.c b/src/util/virfile.c
index e5cf2c5..f45e18f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -681,7 +681,7 @@ static int virFileLoopDeviceOpen(char **dev_name)
 if (virFileLoopDeviceOpenLoopCtl(dev_name, _fd) < 0)
 return -1;
 
-VIR_DEBUG("Return from loop-control got fd %d\n", loop_fd);
+VIR_DEBUG("Return from loop-control got fd %d", loop_fd);
 
 if (loop_fd >= 0)
 return loop_fd;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index bc90c44..fab621b 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -287,7 +287,7 @@ virHashGrow(virHashTablePtr table, size_t size)
 VIR_FREE(oldtable);
 
 #ifdef DEBUG_GROW
-VIR_DEBUG("virHashGrow : from %d to %d, %ld elems\n", oldsize,
+VIR_DEBUG("virHashGrow : from %d to %d, %ld elems", oldsize,
   size, nbElem);
 #endif
 
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 89985b8..f85bd3e 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -473,16 +473,16 @@ virNetDevMacVLanVPortProfileCallback(struct nlmsghdr *hdr,
 case RTM_DELLINK:
 case RTM_SETLINK:
 case RTM_GETLINK:
-VIR_DEBUG(" IFINFOMSG\n");
-VIR_DEBUG("ifi_family = 0x%02x\n",
+VIR_DEBUG(" IFINFOMSG");
+VIR_DEBUG("ifi_family = 0x%02x",
 ((struct ifinfomsg 

[libvirt] [PATCH v2] locking: Add io_timeout to sanlock

[Michal Privoznik]

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

So, if domain loses access to storage, sanlock tries to kill it
after some timeout. So far, the default is 80 seconds. But for
some scenarios this might not be enough. We should allow users to
adjust the timeout according to their needs.

Signed-off-by: Michal Privoznik 
---

diff to v2:
- Check if the new sanlock API is accessible. If not, forbid setting timeout in
  the config file.

 m4/virt-sanlock.m4  |  7 +++
 src/locking/libvirt_sanlock.aug |  1 +
 src/locking/lock_driver_sanlock.c   | 15 +++
 src/locking/sanlock.conf|  7 +++
 src/locking/test_libvirt_sanlock.aug.in |  1 +
 5 files changed, 31 insertions(+)

diff --git a/m4/virt-sanlock.m4 b/m4/virt-sanlock.m4
index c7c0186..d2a607d 100644
--- a/m4/virt-sanlock.m4
+++ b/m4/virt-sanlock.m4
@@ -46,6 +46,13 @@ AC_DEFUN([LIBVIRT_CHECK_SANLOCK],[
 [whether sanlock supports sanlock_inq_lockspace])
 fi
 
+AC_CHECK_LIB([sanlock_client], [sanlock_add_lockspace_timeout],
+ [sanlock_add_lockspace_timeout=yes], 
[sanlock_add_lockspace_timeout=no])
+if test "x$sanlock_add_lockspace_timeout" = "xyes" ; then
+  AC_DEFINE_UNQUOTED([HAVE_SANLOCK_ADD_LOCKSPACE_TIMEOUT], 1,
+[whether Sanlock supports sanlock_add_lockspace_timeout])
+fi
+
 CPPFLAGS="$old_cppflags"
 LIBS="$old_libs"
   fi
diff --git a/src/locking/libvirt_sanlock.aug b/src/locking/libvirt_sanlock.aug
index a78a444..8843590 100644
--- a/src/locking/libvirt_sanlock.aug
+++ b/src/locking/libvirt_sanlock.aug
@@ -22,6 +22,7 @@ module Libvirt_sanlock =
  | int_entry "host_id"
  | bool_entry "require_lease_for_disks"
  | bool_entry "ignore_readonly_and_shared_disks"
+ | int_entry "io_timeout"
  | str_entry "user"
  | str_entry "group"
let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ 
\t\n][^\n]*)?/ . del /\n/ "\n" ]
diff --git a/src/locking/lock_driver_sanlock.c 
b/src/locking/lock_driver_sanlock.c
index e052875..dbda915 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -73,6 +73,7 @@ struct _virLockManagerSanlockDriver {
 int hostID;
 bool autoDiskLease;
 char *autoDiskLeasePath;
+unsigned int io_timeout;
 
 /* under which permissions does sanlock run */
 uid_t user;
@@ -151,6 +152,10 @@ static int virLockManagerSanlockLoadConfig(const char 
*configFile)
 else
 driver->requireLeaseForDisks = !driver->autoDiskLease;
 
+p = virConfGetValue(conf, "io_timeout");
+CHECK_TYPE("io_timeout", VIR_CONF_ULONG);
+if (p) driver->io_timeout = p->l;
+
 p = virConfGetValue(conf, "user");
 CHECK_TYPE("user", VIR_CONF_STRING);
 if (p) {
@@ -338,7 +343,16 @@ static int virLockManagerSanlockSetupLockspace(void)
  * or we can fallback to polling.
  */
  retry:
+#ifdef HAVE_SANLOCK_ADD_LOCKSPACE_TIMEOUT
+if ((rv = sanlock_add_lockspace_timeout(, 0, driver->io_timeout)) < 0) {
+#else
+if (driver->io_timeout) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("unable to use io_timeout with this version of 
sanlock"));
+goto error;
+}
 if ((rv = sanlock_add_lockspace(, 0)) < 0) {
+#endif
 if (-rv == EINPROGRESS && --retries) {
 #ifdef HAVE_SANLOCK_INQ_LOCKSPACE
 /* we have this function which blocks until lockspace change the
@@ -404,6 +418,7 @@ static int virLockManagerSanlockInit(unsigned int version,
 driver->requireLeaseForDisks = true;
 driver->hostID = 0;
 driver->autoDiskLease = false;
+driver->io_timeout = 0;
 driver->user = (uid_t) -1;
 driver->group = (gid_t) -1;
 if (VIR_STRDUP(driver->autoDiskLeasePath, LOCALSTATEDIR 
"/lib/libvirt/sanlock") < 0) {
diff --git a/src/locking/sanlock.conf b/src/locking/sanlock.conf
index e5566ef..3a1a51c 100644
--- a/src/locking/sanlock.conf
+++ b/src/locking/sanlock.conf
@@ -54,6 +54,13 @@
 #require_lease_for_disks = 1
 
 #
+# Sanlock is able to kill qemu processes on IO timeout. By its internal
+# implementation, the current default is 80 seconds. If you need to adjust
+# the value change the following variable. Value of zero means use the
+# default sanlock timeout.
+#io_timeout = 0
+
+#
 # The combination of user and group under which the sanlock
 # daemon runs. Libvirt will chown created files (like
 # content of disk_lease_dir) to make sure sanlock daemon can
diff --git a/src/locking/test_libvirt_sanlock.aug.in 
b/src/locking/test_libvirt_sanlock.aug.in
index ef98ea6..7f66f81 100644
--- a/src/locking/test_libvirt_sanlock.aug.in
+++ b/src/locking/test_libvirt_sanlock.aug.in
@@ -6,5 +6,6 @@ module Test_libvirt_sanlock =
 { "disk_lease_dir" = "/var/lib/libvirt/sanlock" }
 { "host_id" = "1" }
 { "require_lease_for_disks" = "1" }
+{ "io_timeout" = "0" }
 { "user" = "root" }
 { "group" = 

[libvirt] [PATCH v2 2/3] virsh-domain: update attach-interface to support type=hostdev

[Pavel Hrdina]

Adding this feature will allow users to easily attach a hostdev network
interface using PCI passthrough.

The interface can be attached using --type=hostdev and PCI address or
as --source.  This command also allows you to tell, whether the interface
should be managed.

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

Signed-off-by: Pavel Hrdina 
---
 tools/virsh-domain.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 12e85e3..bd00785 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -56,6 +56,7 @@
 #include "virtime.h"
 #include "virtypedparam.h"
 #include "virxml.h"
+#include "virsh-nodedev.h"
 
 /* Gnulib doesn't guarantee SA_SIGINFO support.  */
 #ifndef SA_SIGINFO
@@ -866,6 +867,10 @@ static const vshCmdOptDef opts_attach_interface[] = {
  .type = VSH_OT_BOOL,
  .help = N_("print XML document rather than attach the interface")
 },
+{.name = "managed",
+ .type = VSH_OT_BOOL,
+ .help = N_("libvirt will automatically detach/attach the device from/to 
host")
+},
 {.name = NULL}
 };
 
@@ -931,6 +936,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 bool config = vshCommandOptBool(cmd, "config");
 bool live = vshCommandOptBool(cmd, "live");
 bool persistent = vshCommandOptBool(cmd, "persistent");
+bool managed = vshCommandOptBool(cmd, "managed");
 
 VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
 
@@ -983,7 +989,12 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 }
 
 /* Make XML of interface */
-virBufferAsprintf(, "\n", type);
+virBufferAsprintf(, "\n");
+else
+virBufferAddLit(, ">\n");
 virBufferAdjustIndent(, 2);
 
 switch (typ) {
@@ -995,6 +1006,26 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 case VIR_DOMAIN_NET_TYPE_DIRECT:
 virBufferAsprintf(, "\n", source);
 break;
+case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+{
+struct PCIAddress pciAddr = {0, 0, 0, 0};
+
+if (str2PCIAddress(source, ) < 0) {
+vshError(ctl, _("cannot parse pci address '%s' for network "
+"interface"), source);
+goto cleanup;
+}
+
+virBufferAddLit(, "\n");
+virBufferAdjustIndent(, 2);
+virBufferAsprintf(, "\n",
+  pciAddr.domain, pciAddr.bus,
+  pciAddr.slot, pciAddr.function);
+virBufferAdjustIndent(, -2);
+virBufferAddLit(, "\n");
+break;
+}
 
 case VIR_DOMAIN_NET_TYPE_USER:
 case VIR_DOMAIN_NET_TYPE_ETHERNET:
@@ -1004,7 +1035,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
 case VIR_DOMAIN_NET_TYPE_MCAST:
 case VIR_DOMAIN_NET_TYPE_UDP:
 case VIR_DOMAIN_NET_TYPE_INTERNAL:
-case VIR_DOMAIN_NET_TYPE_HOSTDEV:
 case VIR_DOMAIN_NET_TYPE_LAST:
 vshError(ctl, _("No support for %s in command 'attach-interface'"),
  type);
-- 
2.6.2

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

Re: [libvirt] [PATCH] bhyve: implement domainGetOSType

[Roman Bogorodskiy]

  Michal Privoznik wrote:

> On 20.10.2015 19:37, Roman Bogorodskiy wrote:
> > ---
> >  src/bhyve/bhyve_driver.c | 22 ++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> > index d44cf2c..efba0ae 100644
> > --- a/src/bhyve/bhyve_driver.c
> > +++ b/src/bhyve/bhyve_driver.c
> > @@ -465,6 +465,27 @@ bhyveDomainIsPersistent(virDomainPtr domain)
> >  }
> >  
> >  static char *
> > +bhyveDomainGetOSType(virDomainPtr dom)
> > +{
> > +virDomainObjPtr vm;
> > +char *ret = NULL;
> > +
> > +if (!(vm = bhyveDomObjFromDomain(dom)))
> > +goto cleanup;
> > +
> > +if (virDomainGetOSTypeEnsureACL(dom->conn, vm->def) < 0)
> > +goto cleanup;
> > +
> > +if (VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)) < 0)
> > +goto cleanup;
> > +
> > + cleanup:
> > +if (vm)
> > +virObjectUnlock(vm);
> > +return ret;
> > +}
> > +
> > +static char *
> >  bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
> >  {
> >  virDomainObjPtr vm;
> > @@ -1487,6 +1508,7 @@ static virHypervisorDriver bhyveHypervisorDriver = {
> >  .domainDefineXML = bhyveDomainDefineXML, /* 1.2.2 */
> >  .domainDefineXMLFlags = bhyveDomainDefineXMLFlags, /* 1.2.12 */
> >  .domainUndefine = bhyveDomainUndefine, /* 1.2.2 */
> > +.domainGetOSType = bhyveDomainGetOSType, /* 1.2.21 */
> >  .domainGetXMLDesc = bhyveDomainGetXMLDesc, /* 1.2.2 */
> >  .domainIsActive = bhyveDomainIsActive, /* 1.2.2 */
> >  .domainIsPersistent = bhyveDomainIsPersistent, /* 1.2.2 */
> > 
> 
> 
> ACK

Pushed, thanks!

Roman Bogorodskiy


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

Re: [libvirt] [PATCH] util: implement virProcessGetStartTime on GNU/kFreeBSD

[Roman Bogorodskiy]

  Andrea Bolognani wrote:

> On Tue, 2015-10-27 at 14:54 +0300, Roman Bogorodskiy wrote:
> > > Weak ACK -- I have nowhere to try it and I'm lazy to install such
> > > machine just for trying out this patch, but I can clearly see it
> > > won't
> > > break anything that works right now, so...
> > 
> > FWIW, it also looks fine to me (though I do not have GNU\kFreeBSD
> > boxes
> > as well, only FreeBSD ones). I can build-test on FreeBSD just to be
> > on a
> > safe side, but it doesn't seem like it could break something indeed.
> 
> I'd like to point out that FreeBSD builds are now performed
> regularly, so any breakage affecting FreeBDS should be detected
> pretty quickly.
> 
> https://ci.centos.org/view/libvirt-project/job/libvirt-freebsd/
> 
> Cheers.

That's great!

Actually, I have Jenkins job configured for a while now. It simply pulls
sources from the github mirror and does 'make check syntax-check' 2
times per day.

https://people.freebsd.org/~novel/misc/libvirt_freebsd_ci.png

There are some occasional failures in tests and syntax-check is not
working quite well because of the 'file list too long' problem, still in
TODO list to fix that...

Roman Bogorodskiy


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

Re: [libvirt] [PATCH] Remove new lines from debug messages

[Andrea Bolognani]

On Tue, 2015-10-27 at 16:29 +0100, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  src/nwfilter/nwfilter_gentech_driver.c |  2 +-
>  src/nwfilter/nwfilter_learnipaddr.c| 10 +-
>  src/rpc/virnetsocket.c |  2 +-
>  src/util/virfile.c |  2 +-
>  src/util/virhash.c |  2 +-
>  src/util/virnetdevmacvlan.c| 28 +---
> --
>  src/util/virprocess.c  |  2 +-
>  src/xen/xend_internal.c|  2 +-
>  tests/virhostdevtest.c | 36 +---
> --
>  tests/virnetsockettest.c   |  2 +-
>  tests/virtimetest.c|  2 +-
>  11 files changed, 45 insertions(+), 45 deletions(-)

Looks good, I would squash in the following:

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index f85bd3e..6e00107 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -565,8 +565,7 @@ virNetDevMacVLanVPortProfileCallback(struct
nlmsghdr *hdr,
 }
 if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb_vf_ports,
 ifla_port_policy)) {
-VIR_DEBUG("nested parsing on level 2"
-  " failed");
+VIR_DEBUG("nested parsing on level 2 failed");
 }
 if (tb3[IFLA_PORT_VF]) {
 VIR_DEBUG("IFLA_PORT_VF = %d",

and note in the commit message that you're also removing
trailing full stops from the messages.

I see there are a few instances remaining:

  src/libvirt.c:
VIR_DEBUG("name \"%s\" to URI components:\n"
  src/network/bridge_driver_linux.c:
VIR_DEBUG("%s output:\n%s", PROC_NET_ROUTE, buf);
  src/xenconfig/xen_sxpr.c:
VIR_DEBUG("Formatted sexpr: \n%s", bufout);

plus one more if we also consider warnings:

  src/nwfilter/nwfilter_dhcpsnoop.c:
VIR_WARN("Waiting for snooping threads to terminate: %u\n",

Are those exceptions or did you just miss them? If the
latter, adding a syntax-check for this is probably a good
idea :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

Re: [libvirt] [PATCH] qemu: Resolve agent deadlock

[John Ferlan]

On 10/27/2015 01:00 AM, Peter Krempa wrote:
> On Mon, Oct 26, 2015 at 17:28:57 -0400, John Ferlan wrote:
>>
>>
>> On 10/26/2015 11:37 AM, Peter Krempa wrote:
>>> On Mon, Oct 26, 2015 at 11:06:21 -0400, John Ferlan wrote:
 If we have a shutdown of a VM by a qemu agent while an agent EOF occurs
 at relatively the same time, it's possible that a deadlock occurs depending
 on what happens first.

 Assume qemuProcessHandleAgentEOF runs first, with the vm->lock it clears
 priv->agent, then clears the vm->lock.
>>>
>>> Couldn't we make sure, that qemuProcessHandleAgentEOF clears priv->agent
>>> if and only if it removed the last reference?
>>
>> reference to?  agent?  vm?  via?
> 
> Since all of this is refering to the agent I was refering to it too.
> 

Difficult to be 100% certain from just your comment - no context and no
suggestion for a way to handle this. There is a reference on the vm once
an agent is connected...

>> qemuConnectAgent/qemuAgentOpen takes out a reference to the agent. That
>> is un-referenced by qemuAgentClose. The EnterAgent takes out a reference
>> which is un-referenced during ExitAgent. Adding a new reference just for
>> EOF processing doesn't solve the problem.
>>
>> EOF doesn't do an agent-unlock, it does a vm-lock/unlock, and calls
>> qemuAgentClose which will attempt an agent-lock. The agent may be locked
>> by some other thread and it needs to wait until that reference is cleared.
>>
>> Adding some sort of unref-agent check logic in EOF similar to ExitAgent
>> feels like it'll cause other issues. It seems "backwards" to remove the
>> last reference and avoid everything else that qemuCloseAgent does.
>>
>> I think logically if some sort of unref-agent check logic was added,
>> then qemuCloseAgent could not be called from either EOF or ExitAgent.
>> >From EOF, if the current thread was the last one, then the agent is
>> free'd so qemuCloseAgent shouldn't be called. If the current thread
>> wasn't the last one, then we'd have to wait for the last reference check
>> in ExitAgent, but once that is done, the agent is freed and thus
>> qemuCloseAgent shouldn't be called.
> 
> I wanted to point out that since we do have the 'EnterAgent' and
> 'ExitAgent' helpers, they should be used to do any kind of logic
> required to do the locking and it should not be necessary at any point
> to copy the pointer to priv->agent. Otherwise it creates a really ugly
> usage pattern which requires a separate variable and basically defeats
> the Enter/Exit pattern we use anywhere else.
> 
> Doing so probably will increase the complexity of either the helpers or
> the closing function, but the complexity will not be exposed in a
> repeated pattern through the code. Since we already have the helpers, we
> should use them and not clutter the rest of the code.
> 

Fair enough... I guess I find it "odd" to access a structure after we've
removed the lock on it, while other code/threads can modify that field.
Since the only access is the one thing that was locked/reffed (eg
priv->agent or in the Enter/Exit Monitor cases priv->mon), then for
99.9% of the time it's fine. The 0.1% is a timing window where
priv->agent (or priv->mon) could be set to NULL by a EOF some time after
EnterAgent has increased the ref on priv->agent.

Looking at the ExitMonitor code compared to the ExitAgent code - it
would seem logically ExitMonitor could have the same issue - that is
priv->mon could be set to NULL in it's EOF path during qemuProcessStop,
but that code is headed towards qemuProcessKill and there's many more
instructions that need to be executed before perhaps an ExitMonitor
would run in a separate thread. NB: I had only a brief look at the
ExitMonitor/EOFMonitor logic. If somehow the EOF thread gets all the way
to the "if (priv->mon) qemuMonitorClose(priv->mon); priv->mon = NULL;"
code before the ExitMonitor thread can ObjectUnref(priv->mon), then it
seems we'd have a similar scenario (unlikely, but possible).

> One other option worth checking is moving the stuff happening in
> qemuAgentClose into the destructor for the agent object
> (qemuAgentDispose) which would then auto-call it in the case where
> you remove the last reference. I didn't check thoroughly enough though
> to see whether it's possible.
> 

That seems risky, IMO... It also doesn't solve the issue where the vm's
priv->agent gets set to NULL by the EOF code and the ExitAgent code
cannot Unlock.

So if the problem is that EOF clears priv->agent before calling
qemuAgentClose(), then what happens if we remove (or move) that
clearing? Well, qemuAgentClose will attempt to get the agent-lock, but
needs to wait for the unlock. When ExitAgent is active it executes it's
Unref and Unlock, but will never have the case of !hasRefs. Once it
unlocks the agent, qemuAgentClose code does it's thing and upon return
could set priv->agent = NULL (inside a vm-lock/unlock). However, that
leaves a window where the agent is free'd, but priv->agent isn't 

Re: [libvirt] [PATCH] util: implement virProcessGetStartTime on GNU/kFreeBSD

[Roman Bogorodskiy]

  Martin Kletzander wrote:

> On Thu, Oct 15, 2015 at 01:50:42PM +0200, Pino Toscano wrote:
> >Use the virProcessGetStartTime implementation also when only the kernel
> >is FreeBSD, such as on GNU/kFreeBSD.
> >---
> Weak ACK -- I have nowhere to try it and I'm lazy to install such
> machine just for trying out this patch, but I can clearly see it won't
> break anything that works right now, so...

Works fine on FreeBSD, as expected, so ACKed and pushed.

Roman Bogorodskiy


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

[libvirt] Plan for next release

[Daniel Veillard]

  I'm a bit late, sorry, but I think we should freeze on Thursday
for the next release, and then shoot for middle of next week for the
actual release,
  I hope this last moment warning isn't a problem,

  thanks,

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

[libvirt] [PATCH 0/5] Some qemuProcessStart cleanups

[Jiri Denemark]

Jiri Denemark (5):
  qemu: Use correct type when calling qemuPrepareNVRAM
  qemu: Rename cleanup label in qemuProcessStart
  qemu: Rename ret variable in qemuProcessStart
  qemu: Introduce cleanup label in qemuProcessStart
  qemu: Fix memory leak in qemuProcessStart

 src/qemu/qemu_process.c | 215 
 1 file changed, 106 insertions(+), 109 deletions(-)

-- 
2.6.2

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

[libvirt] [PATCH 3/5] qemu: Rename ret variable in qemuProcessStart

[Jiri Denemark]

Generally, we use "ret" variable for storing the value we are going to
return at the and of a function, but this is not the case in
qemuProcessStart. Let's rename "ret" as "rv".

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 13433f8..ba6d292 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4365,7 +4365,7 @@ int qemuProcessStart(virConnectPtr conn,
  virNetDevVPortProfileOp vmop,
  unsigned int flags)
 {
-int ret;
+int rv;
 off_t pos = -1;
 char ebuf[1024];
 int logfile = -1;
@@ -4846,15 +4846,15 @@ int qemuProcessStart(virConnectPtr conn,
 
 if (virSecurityManagerPreFork(driver->securityManager) < 0)
 goto error;
-ret = virCommandRun(cmd, NULL);
+rv = virCommandRun(cmd, NULL);
 virSecurityManagerPostFork(driver->securityManager);
 
 /* wait for qemu process to show up */
-if (ret == 0) {
+if (rv == 0) {
 if (virPidFileReadPath(priv->pidfile, >pid) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Domain %s didn't show up"), vm->def->name);
-ret = -1;
+rv = -1;
 }
 VIR_DEBUG("QEMU vm=%p name=%s running with pid=%llu",
   vm, vm->def->name, (unsigned long long)vm->pid);
@@ -4921,10 +4921,10 @@ int qemuProcessStart(virConnectPtr conn,
 if (migrateFrom)
 flags |= VIR_QEMU_PROCESS_START_PAUSED;
 
-if (ret == -1) /* The VM failed to start; tear filters before taps */
+if (rv == -1) /* The VM failed to start; tear filters before taps */
 virDomainConfVMNWFilterTeardown(vm);
 
-if (ret == -1) /* The VM failed to start */
+if (rv == -1) /* The VM failed to start */
 goto error;
 
 VIR_DEBUG("Setting cgroup for emulator (if required)");
@@ -4940,8 +4940,8 @@ int qemuProcessStart(virConnectPtr conn,
 goto error;
 
 /* Failure to connect to agent shouldn't be fatal */
-if ((ret = qemuConnectAgent(driver, vm)) < 0) {
-if (ret == -2)
+if ((rv = qemuConnectAgent(driver, vm)) < 0) {
+if (rv == -2)
 goto error;
 
 VIR_WARN("Cannot connect to QEMU guest agent for %s",
-- 
2.6.2

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

Re: [libvirt] [PATCH v2] locking: Add io_timeout to sanlock

[Jiri Denemark]

On Tue, Oct 27, 2015 at 16:29:51 +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1251190
> 
> So, if domain loses access to storage, sanlock tries to kill it
> after some timeout. So far, the default is 80 seconds. But for
> some scenarios this might not be enough. We should allow users to
> adjust the timeout according to their needs.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> diff to v2:
> - Check if the new sanlock API is accessible. If not, forbid setting timeout 
> in
>   the config file.
> 
>  m4/virt-sanlock.m4  |  7 +++
>  src/locking/libvirt_sanlock.aug |  1 +
>  src/locking/lock_driver_sanlock.c   | 15 +++
>  src/locking/sanlock.conf|  7 +++
>  src/locking/test_libvirt_sanlock.aug.in |  1 +
>  5 files changed, 31 insertions(+)
> 
> diff --git a/m4/virt-sanlock.m4 b/m4/virt-sanlock.m4
> index c7c0186..d2a607d 100644
> --- a/m4/virt-sanlock.m4
> +++ b/m4/virt-sanlock.m4
> @@ -46,6 +46,13 @@ AC_DEFUN([LIBVIRT_CHECK_SANLOCK],[
>  [whether sanlock supports sanlock_inq_lockspace])
>  fi
>  
> +AC_CHECK_LIB([sanlock_client], [sanlock_add_lockspace_timeout],
> + [sanlock_add_lockspace_timeout=yes], 
> [sanlock_add_lockspace_timeout=no])
> +if test "x$sanlock_add_lockspace_timeout" = "xyes" ; then
> +  AC_DEFINE_UNQUOTED([HAVE_SANLOCK_ADD_LOCKSPACE_TIMEOUT], 1,
> +[whether Sanlock supports sanlock_add_lockspace_timeout])
> +fi
> +
>  CPPFLAGS="$old_cppflags"
>  LIBS="$old_libs"
>fi
> diff --git a/src/locking/libvirt_sanlock.aug b/src/locking/libvirt_sanlock.aug
> index a78a444..8843590 100644
> --- a/src/locking/libvirt_sanlock.aug
> +++ b/src/locking/libvirt_sanlock.aug
> @@ -22,6 +22,7 @@ module Libvirt_sanlock =
>   | int_entry "host_id"
>   | bool_entry "require_lease_for_disks"
>   | bool_entry "ignore_readonly_and_shared_disks"
> + | int_entry "io_timeout"
>   | str_entry "user"
>   | str_entry "group"
> let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ 
> \t\n][^\n]*)?/ . del /\n/ "\n" ]
> diff --git a/src/locking/lock_driver_sanlock.c 
> b/src/locking/lock_driver_sanlock.c
> index e052875..dbda915 100644
> --- a/src/locking/lock_driver_sanlock.c
> +++ b/src/locking/lock_driver_sanlock.c
> @@ -73,6 +73,7 @@ struct _virLockManagerSanlockDriver {
>  int hostID;
>  bool autoDiskLease;
>  char *autoDiskLeasePath;
> +unsigned int io_timeout;
>  
>  /* under which permissions does sanlock run */
>  uid_t user;
> @@ -151,6 +152,10 @@ static int virLockManagerSanlockLoadConfig(const char 
> *configFile)
>  else
>  driver->requireLeaseForDisks = !driver->autoDiskLease;
>  
> +p = virConfGetValue(conf, "io_timeout");
> +CHECK_TYPE("io_timeout", VIR_CONF_ULONG);
> +if (p) driver->io_timeout = p->l;
> +
>  p = virConfGetValue(conf, "user");
>  CHECK_TYPE("user", VIR_CONF_STRING);
>  if (p) {
> @@ -338,7 +343,16 @@ static int virLockManagerSanlockSetupLockspace(void)
>   * or we can fallback to polling.
>   */
>   retry:
> +#ifdef HAVE_SANLOCK_ADD_LOCKSPACE_TIMEOUT
> +if ((rv = sanlock_add_lockspace_timeout(, 0, driver->io_timeout)) < 
> 0) {
> +#else
> +if (driver->io_timeout) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("unable to use io_timeout with this version of 
> sanlock"));
> +goto error;
> +}
>  if ((rv = sanlock_add_lockspace(, 0)) < 0) {
> +#endif

Ouch, please, don't mix #if and if blocks. The following would be much
better:

#ifdef HAVE_SANLOCK_ADD_LOCKSPACE_TIMEOUT
rv = sanlock_add_lockspace_timeout(, 0, driver->io_timeout);
#else
if (driver->io_timeout) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
   _("unable to use io_timeout with this version of 
sanlock"));
goto error;
}
rv = sanlock_add_lockspace(, 0);
#endif

if (rv < 0) {

Jirka

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

[libvirt] [PATCH v2] Remove new lines from debug messages

[Jiri Denemark]

VIR_DEBUG will automatically add a new line to the message, having "\n"
at the end or at the beginning of the message results in empty lines.

Signed-off-by: Jiri Denemark 
---
 src/nwfilter/nwfilter_dhcpsnoop.c  |  2 +-
 src/nwfilter/nwfilter_gentech_driver.c |  2 +-
 src/nwfilter/nwfilter_learnipaddr.c| 10 +-
 src/rpc/virnetsocket.c |  2 +-
 src/util/virfile.c |  2 +-
 src/util/virhash.c |  2 +-
 src/util/virnetdevmacvlan.c| 26 
 src/util/virprocess.c  |  2 +-
 src/xen/xend_internal.c|  2 +-
 tests/virhostdevtest.c | 36 +-
 tests/virnetsockettest.c   |  2 +-
 tests/virtimetest.c|  2 +-
 12 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
b/src/nwfilter/nwfilter_dhcpsnoop.c
index f05d4a8..bd6d25f 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -2017,7 +2017,7 @@ static void
 virNWFilterSnoopJoinThreads(void)
 {
 while (virAtomicIntGet() != 0) {
-VIR_WARN("Waiting for snooping threads to terminate: %u\n",
+VIR_WARN("Waiting for snooping threads to terminate: %u",
  virAtomicIntGet());
 usleep(1000 * 1000);
 }
diff --git a/src/nwfilter/nwfilter_gentech_driver.c 
b/src/nwfilter/nwfilter_gentech_driver.c
index 701f8d8..5a4cff1 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -540,7 +540,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
 if (rc)
 break;
 } else if (inc) {
-VIR_DEBUG("Following filter %s\n", inc->filterref);
+VIR_DEBUG("Following filter %s", inc->filterref);
 obj = virNWFilterObjFindByName(>nwfilters, inc->filterref);
 if (obj) {
 
diff --git a/src/nwfilter/nwfilter_learnipaddr.c 
b/src/nwfilter/nwfilter_learnipaddr.c
index 5b55055..1adbadb 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -413,7 +413,7 @@ learnIPAddressThread(void *arg)
 handle = pcap_open_live(listen_if, BUFSIZ, 0, PKT_TIMEOUT_MS, errbuf);
 
 if (handle == NULL) {
-VIR_DEBUG("Couldn't open device %s: %s\n", listen_if, errbuf);
+VIR_DEBUG("Couldn't open device %s: %s", listen_if, errbuf);
 req->status = ENODEV;
 goto done;
 }
@@ -448,13 +448,13 @@ learnIPAddressThread(void *arg)
 filter = virBufferContentAndReset();
 
 if (pcap_compile(handle, , filter, 1, 0) != 0) {
-VIR_DEBUG("Couldn't compile filter '%s'.\n", filter);
+VIR_DEBUG("Couldn't compile filter '%s'", filter);
 req->status = EINVAL;
 goto done;
 }
 
 if (pcap_setfilter(handle, ) != 0) {
-VIR_DEBUG("Couldn't set filter '%s'.\n", filter);
+VIR_DEBUG("Couldn't set filter '%s'", filter);
 req->status = EINVAL;
 pcap_freecode();
 goto done;
@@ -626,7 +626,7 @@ learnIPAddressThread(void *arg)
req->filtername,
req->filterparams);
 VIR_DEBUG("Result from applying firewall rules on "
-  "%s with IP addr %s : %d\n", req->ifname, inetaddr, ret);
+  "%s with IP addr %s : %d", req->ifname, inetaddr, ret);
 }
 } else {
 if (showError)
@@ -638,7 +638,7 @@ learnIPAddressThread(void *arg)
 techdriver->applyDropAllRules(req->ifname);
 }
 
-VIR_DEBUG("pcap thread terminating for interface %s\n", req->ifname);
+VIR_DEBUG("pcap thread terminating for interface %s", req->ifname);
 
 virNWFilterUnlockIface(req->ifname);
 
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 5e5f1ab..526d291 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -196,7 +196,7 @@ int virNetSocketCheckProtocols(bool *hasIPv4,
 
 freeaddrinfo(ai);
 
-VIR_DEBUG("Protocols: v4 %d v6 %d\n", *hasIPv4, *hasIPv6);
+VIR_DEBUG("Protocols: v4 %d v6 %d", *hasIPv4, *hasIPv6);
 
 ret = 0;
  cleanup:
diff --git a/src/util/virfile.c b/src/util/virfile.c
index e5cf2c5..f45e18f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -681,7 +681,7 @@ static int virFileLoopDeviceOpen(char **dev_name)
 if (virFileLoopDeviceOpenLoopCtl(dev_name, _fd) < 0)
 return -1;
 
-VIR_DEBUG("Return from loop-control got fd %d\n", loop_fd);
+VIR_DEBUG("Return from loop-control got fd %d", loop_fd);
 
 if (loop_fd >= 0)
 return loop_fd;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index bc90c44..fab621b 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -287,7 +287,7 @@ virHashGrow(virHashTablePtr table, size_t size)
 VIR_FREE(oldtable);
 
 #ifdef 

[libvirt] [libvirt-glib] gobject: Add wrapper virDomainSetTime()

[Zeeshan Ali (Khattak)]

---
 libvirt-gobject/libvirt-gobject-domain.c | 126 +++
 libvirt-gobject/libvirt-gobject-domain.h |  25 ++
 libvirt-gobject/libvirt-gobject.sym  |   9 +++
 3 files changed, 160 insertions(+)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 34eb7ca..26d0cba 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -1886,3 +1886,129 @@ gboolean 
gvir_domain_get_has_current_snapshot(GVirDomain *dom,
 
 return TRUE;
 }
+
+/**
+ * gvir_domain_set_time:
+ * @dom: the domain
+ * @seconds: The seconds since Epoch in UTC.
+ * @nseconds: The microseconds.
+ * @flags: bitwise-OR of #GVirDomainSetTimeFlags.
+ *
+ * This function tries to set guest time to the given value. The time to set
+ * (@seconds and @nseconds) should be in seconds relative to the Epoch of
+ * 1970-01-01 00:00:00 in UTC.
+ *
+ * If you pass #GVIR_DOMAIN_TIME_SYNC as @flags, the time is reset using
+ * domain's RTC and @seconds and @nseconds arguments are ignored.
+ *
+ * Please note that some hypervisors may require guest agent to be configured
+ * and running in order for this function to work.
+ */
+gboolean gvir_domain_set_time(GVirDomain *dom,
+  gint64 seconds,
+  guint nseconds,
+  guint flags,
+  GError **err)
+{
+GVirDomainPrivate *priv;
+int ret;
+
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
+g_return_val_if_fail(flags == 0 || flags & GVIR_DOMAIN_TIME_SYNC, FALSE);
+
+priv = dom->priv;
+ret = virDomainSetTime(priv->handle, seconds, nseconds, flags);
+if (ret < 0) {
+gvir_set_error_literal(err, GVIR_DOMAIN_ERROR,
+   0,
+   "Unable to set domain time");
+return FALSE;
+}
+
+return TRUE;
+}
+
+typedef struct {
+gint64 seconds;
+guint nseconds;
+guint flags;
+} DomainSetTimeData;
+
+static void domain_set_time_data_free(DomainSetTimeData *data)
+{
+g_slice_free(DomainSetTimeData, data);
+}
+
+static void
+gvir_domain_set_time_helper(GTask *task,
+gpointer object,
+gpointer task_data,
+GCancellable *cancellable G_GNUC_UNUSED)
+{
+GVirDomain *dom = GVIR_DOMAIN(object);
+DomainSetTimeData *data = (DomainSetTimeData *) task_data;
+GError *err = NULL;
+
+if (!gvir_domain_set_time(dom,
+  data->seconds,
+  data->nseconds,
+  data->flags,
+  ))
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
+}
+
+/**
+ * gvir_domain_set_time_async:
+ * @dom: the domain
+ * @dom: the domain
+ * @seconds: The seconds since Epoch in UTC.
+ * @nseconds: The microseconds.
+ * @flags: bitwise-OR of #GVirDomainSetTimeFlags.
+ * @cancellable: (allow-none)(transfer none): cancellation object
+ * @callback: (scope async): completion callback
+ * @user_data: (closure): opaque data for callback
+ *
+ * Asynchronous variant of #gvir_domain_set_time.
+ */
+void gvir_domain_set_time_async(GVirDomain *dom,
+gint64 seconds,
+guint nseconds,
+guint flags,
+GCancellable *cancellable,
+GAsyncReadyCallback callback,
+gpointer user_data)
+{
+GTask *task;
+DomainSetTimeData *data;
+
+g_return_if_fail(GVIR_IS_DOMAIN(dom));
+g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
+
+data = g_slice_new0(DomainSetTimeData);
+data->seconds = seconds;
+data->nseconds = nseconds;
+data->flags = flags;
+
+task = g_task_new (G_OBJECT(dom),
+   cancellable,
+   callback,
+   user_data);
+g_task_set_task_data (task, data, 
(GDestroyNotify)domain_set_time_data_free);
+g_task_run_in_thread(task, gvir_domain_set_time_helper);
+
+g_object_unref(task);
+}
+
+gboolean gvir_domain_set_time_finish(GVirDomain *dom,
+ GAsyncResult *result,
+ GError **err)
+{
+g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
+g_return_val_if_fail(g_task_is_valid(result, G_OBJECT(dom)), FALSE);
+g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
+
+return g_task_propagate_boolean(G_TASK(result), err);
+}
diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
b/libvirt-gobject/libvirt-gobject-domain.h
index 4fe381e..fbf7173 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ 

Re: [libvirt] [PATCH] Remove new lines from debug messages

[Jiri Denemark]

On Tue, Oct 27, 2015 at 17:18:40 +0100, Andrea Bolognani wrote:
> On Tue, 2015-10-27 at 16:29 +0100, Jiri Denemark wrote:
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/nwfilter/nwfilter_gentech_driver.c |  2 +-
> >  src/nwfilter/nwfilter_learnipaddr.c| 10 +-
> >  src/rpc/virnetsocket.c |  2 +-
> >  src/util/virfile.c |  2 +-
> >  src/util/virhash.c |  2 +-
> >  src/util/virnetdevmacvlan.c| 28 +---
> > --
> >  src/util/virprocess.c  |  2 +-
> >  src/xen/xend_internal.c|  2 +-
> >  tests/virhostdevtest.c | 36 +---
> > --
> >  tests/virnetsockettest.c   |  2 +-
> >  tests/virtimetest.c|  2 +-
> >  11 files changed, 45 insertions(+), 45 deletions(-)
> 
> Looks good, I would squash in the following:
> 
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index f85bd3e..6e00107 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -565,8 +565,7 @@ virNetDevMacVLanVPortProfileCallback(struct
> nlmsghdr *hdr,
>  }
>  if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb_vf_ports,
>  ifla_port_policy)) {
> -VIR_DEBUG("nested parsing on level 2"
> -  " failed");
> +VIR_DEBUG("nested parsing on level 2 failed");
>  }
>  if (tb3[IFLA_PORT_VF]) {
>  VIR_DEBUG("IFLA_PORT_VF = %d",
> 
> and note in the commit message that you're also removing
> trailing full stops from the messages.

I dropped this hunk completely, removing full stops deserves a separate
patch since we have a lot of them.

> I see there are a few instances remaining:
> 
>   src/libvirt.c:
> VIR_DEBUG("name \"%s\" to URI components:\n"

This is a multiline debug message and the "\n" is not at the end of the
message.

>   src/network/bridge_driver_linux.c:
> VIR_DEBUG("%s output:\n%s", PROC_NET_ROUTE, buf);

Not at the end of the message.

>   src/xenconfig/xen_sxpr.c:
> VIR_DEBUG("Formatted sexpr: \n%s", bufout);

Same here.

> plus one more if we also consider warnings:
> 
>   src/nwfilter/nwfilter_dhcpsnoop.c:
> VIR_WARN("Waiting for snooping threads to terminate: %u\n",

Removed.

> Are those exceptions or did you just miss them? If the
> latter, adding a syntax-check for this is probably a good
> idea :)

Unfortunately, they are exceptions and that's why I didn't add a
syntax-check rule.

Jirka

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

Re: [libvirt] [PATCH] rbd: Remove snapshots if the DELETE_WITH_SNAPSHOTS flag has been provided

[John Ferlan]

On 10/27/2015 10:16 AM, Wido den Hollander wrote:
> When a RBD volume has snapshots it can not be removed.
> 
> This patch introduces a new flag to force volume removal,
> VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS.
> 
> With this flag any existing snapshots will be removed prior to
> removing the volume.
> 
> No existing mechanism in libvirt allowed us to pass such information,
> so that's why a new flag was introduced.
> 
> Signed-off-by: Wido den Hollander 
> ---
>  include/libvirt/libvirt-storage.h |  1 +
>  src/storage/storage_backend_rbd.c | 89 
> +++
>  2 files changed, 90 insertions(+)
> 

With one minor adjustment shown below to initialize snaps, this is now
pushed.

John
> diff --git a/include/libvirt/libvirt-storage.h 
> b/include/libvirt/libvirt-storage.h
> index 453089e..9fc3c2d 100644
> --- a/include/libvirt/libvirt-storage.h
> +++ b/include/libvirt/libvirt-storage.h
> @@ -115,6 +115,7 @@ typedef enum {
>  typedef enum {
>  VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */
>  VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0,  /* Clear all data to zeros 
> (slow) */
> +VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS = 1 << 1, /* Force removal of 
> volume, even if in use */
>  } virStorageVolDeleteFlags;
>  
>  typedef enum {
> diff --git a/src/storage/storage_backend_rbd.c 
> b/src/storage/storage_backend_rbd.c
> index 5ae4713..a37d286 100644
> --- a/src/storage/storage_backend_rbd.c
> +++ b/src/storage/storage_backend_rbd.c
> @@ -421,6 +421,87 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr 
> conn,
>  return ret;
>  }
>  
> +static int virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx,
> +virStoragePoolSourcePtr 
> source,
> +virStorageVolDefPtr vol)
> +{
> +int ret = -1;
> +int r = 0;
> +int max_snaps = 128;
> +int snap_count, protected;
> +size_t i;
> +rbd_snap_info_t *snaps;

snaps = NULL

> +rbd_image_t image = NULL;
> +
> +r = rbd_open(ioctx, vol->name, , NULL);
> +if (r < 0) {
> +   virReportSystemError(-r, _("failed to open the RBD image '%s'"),
> +vol->name);
> +   goto cleanup;
> +}
> +
> +do {
> +if (VIR_ALLOC_N(snaps, max_snaps))
> +goto cleanup;
> +
> +snap_count = rbd_snap_list(image, snaps, _snaps);
> +if (snap_count <= 0)
> +VIR_FREE(snaps);
> +
> +} while (snap_count == -ERANGE);
> +
> +VIR_DEBUG("Found %d snapshots for volume %s/%s", snap_count,
> +  source->name, vol->name);
> +
> +if (snap_count > 0) {
> +for (i = 0; i < snap_count; i++) {
> +if (rbd_snap_is_protected(image, snaps[i].name, )) {
> +virReportSystemError(-r, _("failed to verify if snapshot 
> '%s/%s@%s' is protected"),
> + source->name, vol->name,
> + snaps[i].name);
> +goto cleanup;
> +}
> +
> +if (protected == 1) {
> +VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be "
> +  "unprotected", source->name, vol->name,
> +  snaps[i].name);
> +
> +if (rbd_snap_unprotect(image, snaps[i].name) < 0) {
> +virReportSystemError(-r, _("failed to unprotect snapshot 
> '%s/%s@%s'"),
> + source->name, vol->name,
> + snaps[i].name);
> +goto cleanup;
> +}
> +}
> +
> +VIR_DEBUG("Removing snapshot %s/%s@%s", source->name,
> +  vol->name, snaps[i].name);
> +
> +r = rbd_snap_remove(image, snaps[i].name);
> +if (r < 0) {
> +virReportSystemError(-r, _("failed to remove snapshot 
> '%s/%s@%s'"),
> + source->name, vol->name,
> + snaps[i].name);
> +goto cleanup;
> +}
> +}
> +}
> +
> +ret = 0;
> +
> + cleanup:
> +if (snaps)
> +rbd_snap_list_end(snaps);
> +
> +VIR_FREE(snaps);
> +
> +if (image)
> +rbd_close(image);
> +
> +return ret;
> +}
> +
>  static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
>   virStoragePoolObjPtr pool,
>   virStorageVolDefPtr vol,
> @@ -443,6 +524,14 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr 
> conn,
>  if (virStorageBackendRBDOpenIoCTX(, pool) < 0)
>  goto cleanup;
>  
> +if (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS) {
> +if (virStorageBackendRBDCleanupSnapshots(ptr.ioctx, 
> >def->source,
> + vol) < 0)
> + 

Re: [libvirt] [PATCH] qemu_agent: fix deadlock in qemuProcessHandleAgentEOF

[John Ferlan]

On 10/02/2015 08:17 AM, John Ferlan wrote:
> 
> 
> On 09/26/2015 08:18 AM, Wang Yufei wrote:
>> We shutdown a VM A by qemu agent,meanwhile an agent EOF
>> of VM A happened, there's a chance that deadlock occurred:
>>
>> qemuProcessHandleAgentEOF in main thread
>> A)  priv->agent = NULL; //A happened before B
>>
>> //deadlock when we get agent lock which's held by worker thread
>> qemuAgentClose(agent);
>>
>> qemuDomainObjExitAgent called by qemuDomainShutdownFlags in worker thread
>> B)  hasRefs = virObjectUnref(priv->agent); //priv->agent is NULL, return 
>> false
>>
>> if (hasRefs)
>> virObjectUnlock(priv->agent); //agent lock will not be released here
>>
>> So I close agent first, then set priv->agent NULL to fix the deadlock.
>>
>> Signed-off-by: Wang Yufei 
>> Reviewed-by: Ren Guannan 
>> ---
>>  src/qemu/qemu_process.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
> 
> Interesting - this is the exact opposite of commit id '1020a504' from
> Michal over 3 years ago.
> 
> However, a bit of digging into the claim from the commit message drove
> me to commit id '362d04779c' which removes the domain object lock that
> was the basis of the initial patch.
> 
> While I'm not an expert in the vmagent code, I do note that the
> HandleAgentEOF code checks for !priv->agent and priv->beingDestroyed,
> while the ExitAgent code doesn't necessarily (or directly) check whether
> the priv->agent is still valid (IOW: that nothing else has removed it
> already like the EOF).
> 
> So, while I don't discount that the patch works - I'm wondering whether
> the smarts/logic should be built into ExitAgent to check for
> !priv->agent (and or something else) that would indicate we're already
> on the path of shutdown.
> 
> John
> 
> 

Ironically after spinning a few cycles and generating another patch, it
seems my initial instincts were correct with respect to commit
362d04779c removing the reason/cause for 1020a504 and thus moving the
qemuAgentClose is the correct option.

Of course I'm not sure what caused me to doubt my first thoughts and
start down the path of trying different ways to resolve this. I think I
somehow got it stuck in my head that AgentDestroy still was taking out a
lock. Oh well, I did learn something at least with respect to how this
code works - so that part is good.  I can also answer my own question
with respect whether ExitAgent needs to check priv->agent and/or
priv->beingDestroyed.

With this patch, the EOF code will take out the vm-lock, then attempt to
take out the agent-lock, but will be 'stuck' waiting for the AgentExit
code to run. Once ExitAgent is called, it will remove it's reference and
unlock. I don't believe there's a way for the !hasRefs to be true, so I
suppose it could be removed...

I'll hold off on pushing to allow pkrempa to make any comments.

John


>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index f2586a1..8c9622e 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -150,11 +150,10 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
>>  goto unlock;
>>  }
>>  
>> +qemuAgentClose(agent);
>>  priv->agent = NULL;
>>  
>>  virObjectUnlock(vm);
>> -
>> -qemuAgentClose(agent);
>>  return;
>>  
>>   unlock:
>>
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

Re: [libvirt] [PATCH v2 2/3] virsh-domain: update attach-interface to support type=hostdev

[John Ferlan]

On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
> Adding this feature will allow users to easily attach a hostdev network
> interface using PCI passthrough.
> 
> The interface can be attached using --type=hostdev and PCI address or
> as --source.  This command also allows you to tell, whether the interface
> should be managed.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=997561
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  tools/virsh-domain.c | 34 --
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 12e85e3..bd00785 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -56,6 +56,7 @@
>  #include "virtime.h"
>  #include "virtypedparam.h"
>  #include "virxml.h"
> +#include "virsh-nodedev.h"
>  
>  /* Gnulib doesn't guarantee SA_SIGINFO support.  */
>  #ifndef SA_SIGINFO
> @@ -866,6 +867,10 @@ static const vshCmdOptDef opts_attach_interface[] = {
>   .type = VSH_OT_BOOL,
>   .help = N_("print XML document rather than attach the interface")
>  },
> +{.name = "managed",
> + .type = VSH_OT_BOOL,
> + .help = N_("libvirt will automatically detach/attach the device from/to 
> host")
> +},
>  {.name = NULL}
>  };
>  
> @@ -931,6 +936,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
>  bool config = vshCommandOptBool(cmd, "config");
>  bool live = vshCommandOptBool(cmd, "live");
>  bool persistent = vshCommandOptBool(cmd, "persistent");
> +bool managed = vshCommandOptBool(cmd, "managed");
>  
>  VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
>  
> @@ -983,7 +989,12 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
>  }
>  
>  /* Make XML of interface */
> -virBufferAsprintf(, "\n", type);
> +virBufferAsprintf(, " +
> +if (managed)
> +virBufferAddLit(, " managed='yes'>\n");
> +else
> +virBufferAddLit(, ">\n");
>  virBufferAdjustIndent(, 2);
>  
>  switch (typ) {
> @@ -995,6 +1006,26 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
>  case VIR_DOMAIN_NET_TYPE_DIRECT:
>  virBufferAsprintf(, "\n", source);
>  break;
> +case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +{
> +struct PCIAddress pciAddr = {0, 0, 0, 0};
> +
> +if (str2PCIAddress(source, ) < 0) {
> +vshError(ctl, _("cannot parse pci address '%s' for network "
> +"interface"), source);
> +goto cleanup;
> +}
> +
> +virBufferAddLit(, "\n");
> +virBufferAdjustIndent(, 2);
> +virBufferAsprintf(, " +  " bus='0x%.2x' slot='0x%.2x' 
> function='0x%.1x'/>\n",
> +  pciAddr.domain, pciAddr.bus,
> +  pciAddr.slot, pciAddr.function);
> +virBufferAdjustIndent(, -2);
> +virBufferAddLit(, "\n");
> +break;
> +}
>  
>  case VIR_DOMAIN_NET_TYPE_USER:
>  case VIR_DOMAIN_NET_TYPE_ETHERNET:
> @@ -1004,7 +1035,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
>  case VIR_DOMAIN_NET_TYPE_MCAST:
>  case VIR_DOMAIN_NET_TYPE_UDP:
>  case VIR_DOMAIN_NET_TYPE_INTERNAL:
> -case VIR_DOMAIN_NET_TYPE_HOSTDEV:
>  case VIR_DOMAIN_NET_TYPE_LAST:
>  vshError(ctl, _("No support for %s in command 'attach-interface'"),
>   type);
> 

What happens if someone provides --managed with some other 'typ'?

IOW: If it's only being supported by , then after the switch, a
check should probably occur for "if (managed && typ !=
VIR_DOMAIN_NET_TYPE_HOSTDEV), message, and fail.

I'm not fully clear after reading the bz and the previous review whether
this patch resolves the bz - something to be worked out in the bz for
history's sake though

I think with the adjustment to check whether managed is supplied for non
hostdev, then you have an ACK for what's changed here.

John

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

Re: [libvirt] [PATCH v2 3/3] virsh.pod: update and improve a attach-interface section

[John Ferlan]

On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
> Rewrite the attach-interface section in man page to be more readable and
> add the new hostdev functionality.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  tools/virsh.pod | 82 
> +++--
>  1 file changed, 50 insertions(+), 32 deletions(-)
> 

Why a separate patch? It's related to the previous one and if anyone
ever (ahem) backed ported the other one, they could miss this one... No
strong feeling either way - just curious.

> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 0212e7a..843c293 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode 
> shareable>.
>  [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
>  [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]
>  [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>]
> -[I<--print-xml>]
> -
> -Attach a new network interface to the domain.  I can be
> -I to indicate connection via a libvirt virtual network, or
> -I to indicate connection via a bridge device on the host, or
> -I to indicate connection directly to one of the host's network
> -interfaces or bridges.  I indicates the source of the
> -connection (the name of a network, or of a bridge device, or the
> -host's network interfaces or bridges).  I is used to specify
> -the tap/macvtap device to be used to connect the domain to the
> -source. Names starting with 'vnet' are considered as auto-generated
> -and are blanked out/regenerated each time the interface is attached.
> -I specifies the MAC address of the network interface; if a MAC
> +[I<--managed>] [I<--print-xml>]
> +
> +Attach a new network interface to the domain.
> +
> +B<--type> can be one of the: I to indicate connection via a libvirt
> +virtual network, I to indicate connection via a bridge device
> +on the host, I to indicate connection directly to one of the host's
> +network interfaces or bridges, I to indicate connection using a
> +passthrough of PCI device on the host.

Using a ':' I think is unnecessary unless you somehow generate a real
list such as via "=item * " entries.  In that case you'd have the
following:

+B<--type> can be one of the following:
+
+=over 4
+
+=item * Use I to indicate connection via a libvirt virtual network
+
+=item * Use I to indicate connection via a bridge device on the
host
+
+
+=item * Use I to indicate connection directly to one of the host's
+network interfaces or bridges
+
+=item * Use I to indicate connection using a passthrough of
PCI device
+on the host.
+
+=back

NB: Whether the '--' is required on the type is perhaps a matter of
opinion... Since the command line shown doesn't list --type shouldn't
this still be an 'B'?

> +
> +B<--source> indicates the source of the connection.  The source depends
> +on the type of the interface: I name of the virtual network,
> +I the name of the bridge device, I the name of the host's
> +interface or bridge, I the PCI address of the host's interface
> +formatted as domain:bus:slot.function.

Similar comment/construct here and same comment about '--' for source.

> +
> +B<--target> is used to specify the tap/macvtap device to be used to
> +connect the domain to the source.  Names starting with 'vnet' are
> +considered as auto-generated and are blanked out/regenerated each
> +time the interface is attached.
> +
> +B<--mac> specifies the MAC address of the network interface; if a MAC

B<--target> and B<--mac> seem OK...

>  address is not given, a new address will be automatically generated
>  (and stored in the persistent configuration if "--config" is given on
> -the commandline).  

Re: [libvirt] [PATCH v2 1/3] virsh-nodedev: makes struct and functions for NodeDevice list available

[John Ferlan]

$SUBJ: Expose virshNodeDeviceList{Collect|Free} and virshNodeDeviceList
struct

On 10/27/2015 11:01 AM, Pavel Hrdina wrote:
> Next patch will use those function to collect NodeDevice list and find a
> specific device.  Make functions virshNodeDeviceListCollect() and
> virshNodeDeviceListFree() together with struct virshNodeDeviceList
> available to reuse existing code.
> 

Exposing virshNodeDeviceListCollect, virshNodeDeviceListFree, and
virshNodeDeviceList allows the data returned to be available to other
virsh API's that may need them in the future.

> Signed-off-by: Pavel Hrdina 
> ---
>  tools/virsh-nodedev.c | 16 +---
>  tools/virsh-nodedev.h | 11 +++
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 

OK - all that said, but your future patches don't use these functions,
so is there really any use for this patch yet?  It seems your 2/3 has
removed what was in the 3/4 in your prior series related to calling
virshNodeDeviceListCollect (and noted in your cover letter as being
removed).

I don't oppose the change, but it doesn't seem necessary.

John
> diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
> index cc359e2..26f2c7b 100644
> --- a/tools/virsh-nodedev.c
> +++ b/tools/virsh-nodedev.c
> @@ -194,13 +194,7 @@ virshNodeDeviceSorter(const void *a, const void *b)
>   virNodeDeviceGetName(*nb));
>  }
>  
> -struct virshNodeDeviceList {
> -virNodeDevicePtr *devices;
> -size_t ndevices;
> -};
> -typedef struct virshNodeDeviceList *virshNodeDeviceListPtr;
> -
> -static void
> +void
>  virshNodeDeviceListFree(virshNodeDeviceListPtr list)
>  {
>  size_t i;
> @@ -215,11 +209,11 @@ virshNodeDeviceListFree(virshNodeDeviceListPtr list)
>  VIR_FREE(list);
>  }
>  
> -static virshNodeDeviceListPtr
> +virshNodeDeviceListPtr
>  virshNodeDeviceListCollect(vshControl *ctl,
> - char **capnames,
> - int ncapnames,
> - unsigned int flags)
> +   char **capnames,
> +   int ncapnames,
> +   unsigned int flags)
>  {
>  virshNodeDeviceListPtr list = vshMalloc(ctl, sizeof(*list));
>  size_t i;
> diff --git a/tools/virsh-nodedev.h b/tools/virsh-nodedev.h
> index c64f7df..1d2337b 100644
> --- a/tools/virsh-nodedev.h
> +++ b/tools/virsh-nodedev.h
> @@ -30,4 +30,15 @@
>  
>  extern const vshCmdDef nodedevCmds[];
>  
> +struct virshNodeDeviceList {
> +virNodeDevicePtr *devices;
> +size_t ndevices;
> +};
> +typedef struct virshNodeDeviceList *virshNodeDeviceListPtr;
> +
> +virshNodeDeviceListPtr virshNodeDeviceListCollect(vshControl *ctl,
> +  char **capnames,
> +  int ncapnames,
> +  unsigned int flags);
> +void virshNodeDeviceListFree(virshNodeDeviceListPtr list);
>  #endif /* VIRSH_NODEDEV_H */
> 

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

Re: [libvirt] [PATCH] conf: Add serial target type to ABI stability check

[lhuang]

On 10/27/2015 05:45 PM, Martin Kletzander wrote:

On Wed, Oct 21, 2015 at 03:14:03PM +0800, Luyao Huang wrote:

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

There is no ABI check for serial target type attribute, just
add it.

Signed-off-by: Luyao Huang 
---
src/conf/domain_conf.c | 8 
1 file changed, 8 insertions(+)



ACK && Pushed


Thanks a lot for your review.

Luyao




diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02cc8ad..a31bc05 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17225,6 +17225,14 @@ static bool
virDomainSerialDefCheckABIStability(virDomainChrDefPtr src,
virDomainChrDefPtr dst)
{
+if (src->targetType != dst->targetType) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target serial type %s does not match 
source %s"),

+ virDomainChrSerialTargetTypeToString(dst->targetType),
+ virDomainChrSerialTargetTypeToString(src->targetType));
+return false;
+}
+
if (src->target.port != dst->target.port) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("Target serial port %d does not match source 
%d"),

--
1.8.3.1

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


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

Re: [libvirt] [PATCH] qemu: fix migration flags undefinesource cannot work

[lhuang]

On 10/27/2015 05:45 PM, Martin Kletzander wrote:

On Tue, Oct 27, 2015 at 04:53:59PM +0800, Luyao Huang wrote:

In commit f41be296, we moved vm->persistent check into
qemuDomainRemoveInactive, but we didn't change the vm->persistent
before call qemuDomainRemoveInactive in some place before and just
call it to remove the inactive vm.

Signed-off-by: Luyao Huang 
---
src/qemu/qemu_migration.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)



ACK to all three, pushed now.


Thanks a lot for your review.

Luyao




diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b53491a..2abf2ee 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3912,8 +3912,10 @@ qemuMigrationConfirm(virConnectPtr conn,

qemuMigrationJobFinish(driver, vm);
if (!virDomainObjIsActive(vm)) {
-if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
+if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, 
vm);

+vm->persistent = 0;
+}
qemuDomainRemoveInactive(driver, vm);
}

@@ -5405,8 +5407,10 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,

qemuMigrationJobFinish(driver, vm);
if (!virDomainObjIsActive(vm) && ret == 0) {
-if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
+if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, 
vm);

+vm->persistent = 0;
+}
qemuDomainRemoveInactive(driver, vm);
}

--
1.8.3.1

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


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

Re: [libvirt] [PATCH 0/5] Some qemuProcessStart cleanups

[John Ferlan]

On 10/27/2015 11:27 AM, Jiri Denemark wrote:
> Jiri Denemark (5):
>   qemu: Use correct type when calling qemuPrepareNVRAM
>   qemu: Rename cleanup label in qemuProcessStart
>   qemu: Rename ret variable in qemuProcessStart
>   qemu: Introduce cleanup label in qemuProcessStart
>   qemu: Fix memory leak in qemuProcessStart
> 
>  src/qemu/qemu_process.c | 215 
> 
>  1 file changed, 106 insertions(+), 109 deletions(-)
> 

ACK series

John

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

[libvirt] [PATCH] rbd: Remove snapshots if the DELETE_WITH_SNAPSHOTS flag has been provided

[Wido den Hollander]

When a RBD volume has snapshots it can not be removed.

This patch introduces a new flag to force volume removal,
VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS.

With this flag any existing snapshots will be removed prior to
removing the volume.

No existing mechanism in libvirt allowed us to pass such information,
so that's why a new flag was introduced.

Signed-off-by: Wido den Hollander 
---
 include/libvirt/libvirt-storage.h |  1 +
 src/storage/storage_backend_rbd.c | 89 +++
 2 files changed, 90 insertions(+)

diff --git a/include/libvirt/libvirt-storage.h 
b/include/libvirt/libvirt-storage.h
index 453089e..9fc3c2d 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -115,6 +115,7 @@ typedef enum {
 typedef enum {
 VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only(fast) */
 VIR_STORAGE_VOL_DELETE_ZEROED = 1 << 0,  /* Clear all data to zeros (slow) 
*/
+VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS = 1 << 1, /* Force removal of 
volume, even if in use */
 } virStorageVolDeleteFlags;
 
 typedef enum {
diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 5ae4713..a37d286 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -421,6 +421,87 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr 
conn,
 return ret;
 }
 
+static int virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx,
+virStoragePoolSourcePtr source,
+virStorageVolDefPtr vol)
+{
+int ret = -1;
+int r = 0;
+int max_snaps = 128;
+int snap_count, protected;
+size_t i;
+rbd_snap_info_t *snaps;
+rbd_image_t image = NULL;
+
+r = rbd_open(ioctx, vol->name, , NULL);
+if (r < 0) {
+   virReportSystemError(-r, _("failed to open the RBD image '%s'"),
+vol->name);
+   goto cleanup;
+}
+
+do {
+if (VIR_ALLOC_N(snaps, max_snaps))
+goto cleanup;
+
+snap_count = rbd_snap_list(image, snaps, _snaps);
+if (snap_count <= 0)
+VIR_FREE(snaps);
+
+} while (snap_count == -ERANGE);
+
+VIR_DEBUG("Found %d snapshots for volume %s/%s", snap_count,
+  source->name, vol->name);
+
+if (snap_count > 0) {
+for (i = 0; i < snap_count; i++) {
+if (rbd_snap_is_protected(image, snaps[i].name, )) {
+virReportSystemError(-r, _("failed to verify if snapshot 
'%s/%s@%s' is protected"),
+ source->name, vol->name,
+ snaps[i].name);
+goto cleanup;
+}
+
+if (protected == 1) {
+VIR_DEBUG("Snapshot %s/%s@%s is protected needs to be "
+  "unprotected", source->name, vol->name,
+  snaps[i].name);
+
+if (rbd_snap_unprotect(image, snaps[i].name) < 0) {
+virReportSystemError(-r, _("failed to unprotect snapshot 
'%s/%s@%s'"),
+ source->name, vol->name,
+ snaps[i].name);
+goto cleanup;
+}
+}
+
+VIR_DEBUG("Removing snapshot %s/%s@%s", source->name,
+  vol->name, snaps[i].name);
+
+r = rbd_snap_remove(image, snaps[i].name);
+if (r < 0) {
+virReportSystemError(-r, _("failed to remove snapshot 
'%s/%s@%s'"),
+ source->name, vol->name,
+ snaps[i].name);
+goto cleanup;
+}
+}
+}
+
+ret = 0;
+
+ cleanup:
+if (snaps)
+rbd_snap_list_end(snaps);
+
+VIR_FREE(snaps);
+
+if (image)
+rbd_close(image);
+
+return ret;
+}
+
 static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
  virStoragePoolObjPtr pool,
  virStorageVolDefPtr vol,
@@ -443,6 +524,14 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr 
conn,
 if (virStorageBackendRBDOpenIoCTX(, pool) < 0)
 goto cleanup;
 
+if (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS) {
+if (virStorageBackendRBDCleanupSnapshots(ptr.ioctx, >def->source,
+ vol) < 0)
+goto cleanup;
+}
+
+VIR_DEBUG("Removing volume %s/%s", pool->def->source.name, vol->name);
+
 r = rbd_remove(ptr.ioctx, vol->name);
 if (r < 0 && (-r) != ENOENT) {
 virReportSystemError(-r, _("failed to remove volume '%s/%s'"),
-- 
1.9.1

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

Re: [libvirt] [PATCH 1/4] virsh-domain: use correct base for virStrToLong_ui

[Pavel Hrdina]

On Mon, Oct 26, 2015 at 09:48:20AM -0400, Laine Stump wrote:
> On 10/26/2015 05:53 AM, Pavel Hrdina wrote:
> > The current situation is not ideal, it's not documented anywhere and for 
> > users
> > you can only try the command and see what happens.  Yes, it can and probably
> > will break some scripts for some users, but I think we should make it 
> > somehow
> > consistent and document it properly how to format it.
> >
> > The output of nodedev-dumpxml should be definitely fixed to print the PCI
> > address using only hex numbers.
> >
> > For the parsing part, this code is currently used only for 'attach-disk' and
> > the 'attach-interface' will be a new functionality and we can easily 
> > restrict
> > the format of PCI address to be provided only in hex numbers with '0x' 
> > prefix.
> 
> /me wakes up Monday morning, reads this paragraph, and realizes that he 
> should have looked at the entire patch before commenting :-/
> 
> Yeah, forget everything I said. I hadn't scrolled all the way to the 
> bottom of the patch, but just assumed from the commit message that it 
> was going to fix the base at 16 for the XML parser too. Pretty stupid of 
> me - sorry for all the noise over nothing.
> 
> Definitely any time someone is writing a unified PCI address 
> (:BB:SS.F) I think we can safely assume/require they are using hex. 
> Really, I'm surprised this didn't trip anyone up before now.
> 
> ACK.

Thanks :) pushed now

Pavel

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

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

Re: [libvirt] [PATCH] locking: Add io_timeout to sanlock

[Michal Privoznik]

On 27.10.2015 10:23, Martin Kletzander wrote:
> On Fri, Oct 23, 2015 at 01:37:54PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1251190
>>
>> So, if domain loses access to storage, sanlock tries to kill it
>> after some timeout. So far, the default is 80 seconds. But for
>> some scenarios this might not be enough. We should allow users to
>> adjust the timeout according to their needs.
>>
> 
> Shouldn't we check for whether the current sanlock version supports
> that?  Or require new enough version in configure/libvirt.spec?  I
> understand the _timeout option was added later then the previous one.
> 
> Otherwise looks good, if you convince me that there is no need for the
> check then it can go in as it is.

The _timeout was added in e174a7e33728ba3ad587546693612476a081735d
(sanlock-2.5~5) which dates back to Aug 2012. So I'd say since we are
more than three years since then we are safe. But if you would feel
safer with an explicit check I can do that.

Michal

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