[libvirt] [PATCH v2 0/3] qemu: don't duplicate suspended events and state changes

2019-02-07 Thread Nikolay Shirokovskiy
Patches 1 and 2 are already Reviewed-by: John. Patch 3 needs Peter comments.

Diff from v1:

- minor rebase changes
- minor changes according to review

[1] PATCH v1 : 
https://www.redhat.com/archives/libvir-list/2018-October/msg00591.html

Nikolay Shirokovskiy (3):
  qemu: Pass stop reason from qemuProcessStopCPUs to stop handler
  qemu: Map suspended state reason to suspended event detail
  qemu: Don't duplicate suspend events and state changes

 src/qemu/qemu_domain.c| 34 ++
 src/qemu/qemu_domain.h|  7 +++
 src/qemu/qemu_driver.c| 26 +++---
 src/qemu/qemu_migration.c | 42 ++
 src/qemu/qemu_migration.h |  4 
 src/qemu/qemu_process.c   | 38 +-
 6 files changed, 75 insertions(+), 76 deletions(-)

-- 
1.8.3.1

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


[libvirt] [PATCH v2 3/3] qemu: Don't duplicate suspend events and state changes

2019-02-07 Thread Nikolay Shirokovskiy
Since the STOP event handler can use the pausedReason as sent to
qemuProcessStopCPUs, we no longer need to send duplicate suspended
lifecycle events because we know what caused the stop along with extra
details. This processing allows us to also remove the duplicated state
change from qemuProcessStopCPUs.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/qemu/qemu_driver.c| 26 +++---
 src/qemu/qemu_migration.c | 42 ++
 src/qemu/qemu_migration.h |  4 
 src/qemu/qemu_process.c   |  6 +-
 4 files changed, 14 insertions(+), 64 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 427c1d0..78079ef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1790,10 +1790,8 @@ static int qemuDomainSuspend(virDomainPtr dom)
 virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
 int ret = -1;
-virObjectEventPtr event = NULL;
 qemuDomainObjPrivatePtr priv;
 virDomainPausedReason reason;
-int eventDetail;
 int state;
 virQEMUDriverConfigPtr cfg = NULL;
 
@@ -1812,16 +1810,12 @@ static int qemuDomainSuspend(virDomainPtr dom)
 if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
 
-if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
+if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT)
 reason = VIR_DOMAIN_PAUSED_MIGRATION;
-eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
-} else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT) {
+else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT)
 reason = VIR_DOMAIN_PAUSED_SNAPSHOT;
-eventDetail = -1; /* don't create lifecycle events when doing snapshot 
*/
-} else {
+else
 reason = VIR_DOMAIN_PAUSED_USER;
-eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
-}
 
 state = virDomainObjGetState(vm, NULL);
 if (state == VIR_DOMAIN_PMSUSPENDED) {
@@ -1831,12 +1825,6 @@ static int qemuDomainSuspend(virDomainPtr dom)
 } else if (state != VIR_DOMAIN_PAUSED) {
 if (qemuProcessStopCPUs(driver, vm, reason, QEMU_ASYNC_JOB_NONE) < 0)
 goto endjob;
-
-if (eventDetail >= 0) {
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- eventDetail);
-}
 }
 if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 
0)
 goto endjob;
@@ -1848,7 +1836,6 @@ static int qemuDomainSuspend(virDomainPtr dom)
  cleanup:
 virDomainObjEndAPI(&vm);
 
-virObjectEventStateQueue(driver->domainEventState, event);
 virObjectUnref(cfg);
 return ret;
 }
@@ -16467,13 +16454,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 VIR_DOMAIN_PAUSED_FROM_SNAPSHOT,
 QEMU_ASYNC_JOB_START) < 0)
 goto endjob;
-/* Create an event now in case the restore fails, so
- * that user will be alerted that they are now paused.
- * If restore later succeeds, we might replace this. */
-detail = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- detail);
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("guest unexpectedly quit"));
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 1433b2c..2e7e356 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1269,29 +1269,6 @@ qemuMigrationSrcIsSafe(virDomainDefPtr def,
 return true;
 }
 
-/** qemuMigrationSrcSetOffline
- * Pause domain for non-live migration.
- */
-int
-qemuMigrationSrcSetOffline(virQEMUDriverPtr driver,
-   virDomainObjPtr vm)
-{
-int ret;
-VIR_DEBUG("driver=%p vm=%p", driver, vm);
-ret = qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_MIGRATION,
-  QEMU_ASYNC_JOB_MIGRATION_OUT);
-if (ret == 0) {
-virObjectEventPtr event;
-
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED);
-virObjectEventStateQueue(driver->domainEventState, event);
-}
-
-return ret;
-}
-
 
 void
 qemuMigrationAnyPostcopyFailed(virQEMUDriverPtr driver,
@@ -1314,19 +1291,10 @@ qemuMigrationAnyPostcopyFailed(virQEMUDriverPtr driver,
  "leaving the domain paused", vm->def->name);
 
 if (state == VIR_DOMAIN_RUNNING) {
-virObjectEventPtr event;
-
 if (qemuProcessStopCPUs(driver

[libvirt] [PATCH v2 1/3] qemu: Pass stop reason from qemuProcessStopCPUs to stop handler

2019-02-07 Thread Nikolay Shirokovskiy
Similar to commit [1] which saves and passes the running reason to
the RESUME event handler, during qemuProcessStopCPUs let's save and pass
the pause reason in the domain private data so that the STOP event
handler can use it.

[1] 5dab984ed : qemu: Pass running reason to RESUME event handler

Signed-off-by: Nikolay Shirokovskiy 
---
 src/qemu/qemu_domain.h  |  4 
 src/qemu/qemu_process.c | 15 ---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index fe47417..230c1e1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -369,6 +369,10 @@ struct _qemuDomainObjPrivate {
  * RESUME event handler to use it */
 virDomainRunningReason runningReason;
 
+/* qemuProcessStopCPUs stores the reason for pausing vCPUs here for the
+ * STOP event handler to use it */
+virDomainPausedReason pausedReason;
+
 /* true if libvirt remembers the original owner for files */
 bool rememberOwner;
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0583eb0..6f21962 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -641,14 +641,17 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 {
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event = NULL;
-virDomainPausedReason reason = VIR_DOMAIN_PAUSED_UNKNOWN;
+virDomainPausedReason reason;
 virDomainEventSuspendedDetailType detail = 
VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+qemuDomainObjPrivatePtr priv = vm->privateData;
 
 virObjectLock(vm);
+
+reason = priv->pausedReason;
+priv->pausedReason = VIR_DOMAIN_PAUSED_UNKNOWN;
+
 if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
-qemuDomainObjPrivatePtr priv = vm->privateData;
-
 if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
 if (priv->job.current->status ==
 QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
@@ -3231,6 +3234,8 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver,
 
 VIR_FREE(priv->lockState);
 
+priv->pausedReason = reason;
+
 if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
 goto cleanup;
 
@@ -3253,6 +3258,9 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver,
 VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
 
  cleanup:
+if (ret < 0)
+priv->pausedReason = VIR_DOMAIN_PAUSED_UNKNOWN;
+
 return ret;
 }
 
@@ -6099,6 +6107,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
 priv->monError = false;
 priv->monStart = 0;
 priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN;
+priv->pausedReason = VIR_DOMAIN_PAUSED_UNKNOWN;
 
 VIR_DEBUG("Updating guest CPU definition");
 if (qemuProcessUpdateGuestCPU(vm->def, priv->qemuCaps, caps, flags) < 0)
-- 
1.8.3.1

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


[libvirt] [PATCH v2 2/3] qemu: Map suspended state reason to suspended event detail

2019-02-07 Thread Nikolay Shirokovskiy
Map is based on existing cases in code where we send suspended
event after changing domain state to paused.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/qemu/qemu_domain.c  | 34 ++
 src/qemu/qemu_domain.h  |  3 +++
 src/qemu/qemu_process.c | 17 -
 3 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b6c1a0e..bc4dc5b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -13926,3 +13926,37 @@ 
qemuDomainDiskIsMissingLocalOptional(virDomainDiskDefPtr disk)
virStorageSourceIsLocalStorage(disk->src) && disk->src->path &&
!virFileExists(disk->src->path);
 }
+
+
+virDomainEventSuspendedDetailType
+qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason)
+{
+switch (reason) {
+case VIR_DOMAIN_PAUSED_MIGRATION:
+return VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
+
+case VIR_DOMAIN_PAUSED_FROM_SNAPSHOT:
+return VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
+
+case VIR_DOMAIN_PAUSED_POSTCOPY_FAILED:
+return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY_FAILED;
+
+case VIR_DOMAIN_PAUSED_POSTCOPY:
+return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
+
+case VIR_DOMAIN_PAUSED_UNKNOWN:
+case VIR_DOMAIN_PAUSED_USER:
+case VIR_DOMAIN_PAUSED_SAVE:
+case VIR_DOMAIN_PAUSED_DUMP:
+case VIR_DOMAIN_PAUSED_IOERROR:
+case VIR_DOMAIN_PAUSED_WATCHDOG:
+case VIR_DOMAIN_PAUSED_SHUTTING_DOWN:
+case VIR_DOMAIN_PAUSED_SNAPSHOT:
+case VIR_DOMAIN_PAUSED_CRASHED:
+case VIR_DOMAIN_PAUSED_STARTING_UP:
+case VIR_DOMAIN_PAUSED_LAST:
+break;
+}
+
+return VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 230c1e1..78abc14 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1113,4 +1113,7 @@ qemuDomainIsUsingNoShutdown(qemuDomainObjPrivatePtr priv);
 bool
 qemuDomainDiskIsMissingLocalOptional(virDomainDiskDefPtr disk);
 
+virDomainEventSuspendedDetailType
+qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason);
+
 #endif /* LIBVIRT_QEMU_DOMAIN_H */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6f21962..e0d5320 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -642,7 +642,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event = NULL;
 virDomainPausedReason reason;
-virDomainEventSuspendedDetailType detail = 
VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
+virDomainEventSuspendedDetailType detail;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 
@@ -653,18 +653,17 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 
 if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
 if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
-if (priv->job.current->status ==
-QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
+if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
 reason = VIR_DOMAIN_PAUSED_POSTCOPY;
-detail = VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
-} else {
+else
 reason = VIR_DOMAIN_PAUSED_MIGRATION;
-detail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
-}
 }
 
-VIR_DEBUG("Transitioned guest %s to paused state, reason %s",
-  vm->def->name, virDomainPausedReasonTypeToString(reason));
+detail = qemuDomainPausedReasonToSuspendedEvent(reason);
+VIR_DEBUG("Transitioned guest %s to paused state, "
+  "reason %s, event detail %d",
+  vm->def->name, virDomainPausedReasonTypeToString(reason),
+  detail);
 
 if (priv->job.current)
 ignore_value(virTimeMillisNow(&priv->job.current->stopped));
-- 
1.8.3.1

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


Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread Erik Skultety
On Thu, Feb 07, 2019 at 12:12:14PM -0500, John Ferlan wrote:
>
>
> On 2/7/19 11:54 AM, Andrea Bolognani wrote:
> > On Thu, 2019-02-07 at 17:36 +0100, Erik Skultety wrote:
> >> On Thu, Feb 07, 2019 at 05:11:24PM +0100, Andrea Bolognani wrote:
> >>> On Thu, 2019-02-07 at 16:55 +0100, Erik Skultety wrote:
>  On Thu, Feb 07, 2019 at 04:24:13PM +0100, Andrea Bolognani wrote:
> > Please keep the semicolon! If a macro is used like a function, then
> > its call sites should also look like those of a function.
> 
>  Except VIR_DEFINE_AUTOPTR_FUNC macro is not used like a function, it 
>  defines a
>  function.
> >>>
> >>> Sure, and the way you make that happen is by writing
> >>>
> >>>   MACRO_NAME(argument_one, argument_two);
> >>>
> >>> How is that not using it like a function? :)
> >>
> >> I may need to replace my dictionary, because the way I understand the
> >> expression "like a function" is that the macro is called like function and 
> >> it
> >> behaves like a function, i.e. returns a value, IOW by using the macro its
> >> expansion will perform the usual set of operation on the stack that a 
> >> function
> >> call involves (push parameters, return address, jump into function,
> >> pop the return value...)
> >
> > I guess abort() is not a function either then, since it doesn't have
> > any parameters to push or values to return! :P
> >
> > Anyway, the point is that we have already started mandating the use
> > of semicolon after other macros that expand to definitions, such as
> > VIR_ENUM_DECL(), VIR_ENUM_IMPL(), and VIR_ONCE_GLOBAL_INIT(): we
> > should do the same for VIR_DEFINE_AUTOPTR_FUNC() and increase
> > consistency instead of pushing in the opposite direction.
> >
>
> Since the issue is consistency, how about a patch that adds the ; to the
> existing VIR_DEFINE_AUTOPTR_FUNC's that don't have it?  Ironically, I
> found one that has it:
>
> VIR_DEFINE_AUTOPTR_FUNC(virSEVCapability, virSEVCapabilitiesFree);

That sounds reasonable, I didn't want to end up with one set of macros
following one direction and another following other one, so go ahead ;).

Erik

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


Re: [libvirt] [PATCH v2 31/32] qemu: Use the 'device_id' property of SCSI disks to avoid regressing

2019-02-07 Thread Peter Krempa
On Thu, Feb 07, 2019 at 18:15:06 +0100, Ján Tomko wrote:
> On Mon, Feb 04, 2019 at 04:47:04PM +0100, Peter Krempa wrote:
> > QEMU accidentally exposed the id of -drive (or same value as disk
> > serial, if provided) in one of the identifiers visible from the guest.
> > 
> > To avoid regression in case when -blockdev will be used we need to
> > always specify it ourselves.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> > src/qemu/qemu_command.c   | 22 +++
> > .../controller-virtio-scsi.x86_64-latest.args | 20 -
> > .../disk-cache.x86_64-latest.args |  4 ++--
> > .../disk-scsi-device-auto.x86_64-latest.args  |  3 ++-
> > .../disk-scsi.x86_64-latest.args  | 16 --
> > .../disk-shared.x86_64-latest.args|  5 +++--
> > ...threads-virtio-scsi-pci.x86_64-latest.args |  4 ++--
> > 7 files changed, 50 insertions(+), 24 deletions(-)
> > 
> > @@ -2021,6 +2037,12 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
> >   disk->info.addr.drive.target,
> >   disk->info.addr.drive.unit);
> > }
> > +
> > +if (scsiVPDDeviceId) {
> > +virBufferAddLit(&opt, ",device_id=");
> > +virBufferEscape(&opt, '\\', " ", "%s", scsiVPDDeviceId);
> 
> commas should be escaped, not spaces

Hmm, I stole this from disk serial formatting as it's passed here when
it's present. I'll need to check first whether serial also does not need
a tweak.


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

[libvirt] [tck PATCH 4/4] nwfilter: allow for ebtables *not* removing leading 0 from mac addresses

2019-02-07 Thread Laine Stump
The ebtables command in RHEL8 prints 00 in a MAC address as "00",
unlike e.g. Fedora 29, which prints it as "0". Allow for both.

Signed-off-by: Laine Stump 
---
 scripts/nwfilter/100-ping-still-working.t | 4 ++--
 scripts/nwfilter/210-no-mac-spoofing.t| 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/nwfilter/100-ping-still-working.t 
b/scripts/nwfilter/100-ping-still-working.t
index a88eb02..656722d 100644
--- a/scripts/nwfilter/100-ping-still-working.t
+++ b/scripts/nwfilter/100-ping-still-working.t
@@ -76,9 +76,9 @@ diag "ip is $guestip";
 my $ebtables = (-e '/sbin/ebtables') ? '/sbin/ebtables' : '/usr/sbin/ebtables';
 my $ebtable = `$ebtables -L;$ebtables -t nat -L`;
 diag $ebtable;
-# ebtables shortens :00: to :0: so we need to do that too
+# ebtables *might* shorten :00: to :0: so we need to allow for both when 
searching
 $_ = $mac;
-s/00/0/g;
+s/0([0-9])/0{0,1}$1/g;
 ok($ebtable =~ $_, "check ebtables entry");
 
 # ping guest1
diff --git a/scripts/nwfilter/210-no-mac-spoofing.t 
b/scripts/nwfilter/210-no-mac-spoofing.t
index 78c500c..95f003a 100644
--- a/scripts/nwfilter/210-no-mac-spoofing.t
+++ b/scripts/nwfilter/210-no-mac-spoofing.t
@@ -81,9 +81,9 @@ diag "guest ip is $guestip";
 my $ebtables = (-e '/sbin/ebtables') ? '/sbin/ebtables' : '/usr/sbin/ebtables';
 my $ebtable = `$ebtables -L;$ebtables -t nat -L`;
 diag $ebtable;
-# ebtables shortens :00: to :0: so we need to do that too
+# ebtables *might* shorten :00: to :0: so we need to allow for both when 
searching
 $_ = $mac;
-s/00/0/g; 
+s/0([0-9])/0{0,1}$1/g;
 ok($ebtable =~ $_, "check ebtables entry");
 
 my $macfalse = "52:54:00:f9:21:22";
-- 
2.20.1

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


[libvirt] [tck PATCH 3/4] networks: remove stray use of brctl command

2019-02-07 Thread Laine Stump
brctl has been deprecated for a long time, and distros are starting to
remove it. "ip link blah type bridge" should be used instead.

Signed-off-by: Laine Stump 
---
 scripts/networks/340-guest-network-bridge.t | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/networks/340-guest-network-bridge.t 
b/scripts/networks/340-guest-network-bridge.t
index e5db0ff..498bcb6 100644
--- a/scripts/networks/340-guest-network-bridge.t
+++ b/scripts/networks/340-guest-network-bridge.t
@@ -36,9 +36,9 @@ my $conn = eval { $tck->setup(); };
 BAIL_OUT "failed to setup test harness: $@" if $@;
 END { $tck->cleanup if $tck; }
 
-((system "brctl addbr tck") == 0) or die "cannot create bridge 'tck'";
+((system "ip link add name tck type bridge") == 0) or die "cannot create 
bridge 'tck'";
 
-END { system "brctl delbr tck" }
+END { system "ip link del tck" }
 
 my $b = Sys::Virt::TCK::NetworkBuilder->new(name => "tck");
 $b->bridge("tck");
-- 
2.20.1

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


[libvirt] [tck PATCH 1/4] storage: skip qcow1 tests when qcow1 isn't supported by qemu-img

2019-02-07 Thread Laine Stump
RHEL8 has dropped support for qcow1 format images, so skip the tests
related to creating/cloning qcow1 images (based on the output of
qemu-img -help).

Signed-off-by: Laine Stump 
---
 scripts/storage/100-create-vol-dir.t | 22 -
 scripts/storage/200-clone-vol-dir.t  | 48 
 2 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/scripts/storage/100-create-vol-dir.t 
b/scripts/storage/100-create-vol-dir.t
index 952012c..6262d69 100644
--- a/scripts/storage/100-create-vol-dir.t
+++ b/scripts/storage/100-create-vol-dir.t
@@ -99,18 +99,24 @@ lives_ok(sub { $vol->delete(0) }, "deleted volume");
 
 
 
-ok_volume(sub { $vol = $pool->create_volume($volqcow1xml) }, "create qcow 
volume");
+SKIP: {
+if (`qemu-img -help` !~ "^Supported formats: .* qcow ") {
+skip "qcow1 format not supported", 4;
+}
 
-$path = xpath($vol, "string(/volume/target/path)");
-$st = stat($path);
+ok_volume(sub { $vol = $pool->create_volume($volqcow1xml) }, "create qcow 
volume");
 
-ok($st, "path $path exists");
+$path = xpath($vol, "string(/volume/target/path)");
+$st = stat($path);
 
-# Don't know exactly how large a qcow1 empty file is, but it
-# should be quite small :-)
-ok($st->size < 1024*1024, "basic qcow1 header is allocated");
+ok($st, "path $path exists");
 
-lives_ok(sub { $vol->delete(0) }, "deleted volume");
+# Don't know exactly how large a qcow1 empty file is, but it
+# should be quite small :-)
+ok($st->size < 1024*1024, "basic qcow1 header is allocated");
+
+lives_ok(sub { $vol->delete(0) }, "deleted volume");
+}
 
 
 
diff --git a/scripts/storage/200-clone-vol-dir.t 
b/scripts/storage/200-clone-vol-dir.t
index cc0daba..787564f 100644
--- a/scripts/storage/200-clone-vol-dir.t
+++ b/scripts/storage/200-clone-vol-dir.t
@@ -106,39 +106,45 @@ diag "Now testing cloning of various formats";
 my @formats = qw(raw qcow qcow2 vmdk vpc);
 
 foreach my $format (@formats) {
-diag "Cloning source volume to $format format";
-my $volclonexml = $tck->generic_volume("tck$format", $format, 
((1024*1024*50)+4096))->as_xml;
+SKIP: {
+if (($format eq "qcow") and (`qemu-img -help` !~ "^Supported formats: 
.* qcow ")) {
+skip "qcow1 format not supported", 9;
+}
 
-my $clone;
-ok_volume(sub { $clone = $pool->clone_volume($volclonexml, $vol) }, "clone 
to $format volume");
+diag "Cloning source volume to $format format";
+my $volclonexml = $tck->generic_volume("tck$format", $format, 
((1024*1024*50)+4096))->as_xml;
 
-$path = xpath($clone, "string(/volume/target/path)");
-$st = stat($path);
-ok($st, "path $path exists");
-ok($st->size >= ((1024*1024*50)+4096), "size is at least 50M");
+my $clone;
+ok_volume(sub { $clone = $pool->clone_volume($volclonexml, $vol) }, 
"clone to $format volume");
 
+$path = xpath($clone, "string(/volume/target/path)");
+$st = stat($path);
+ok($st, "path $path exists");
+ok($st->size >= ((1024*1024*50)+4096), "size is at least 50M");
 
-diag "Cloning cloned volume back to raw format";
-my $voldstxml = $tck->generic_volume("tckdst", "raw", 
((1024*1024*50)+4096))->as_xml;
-my $result;
-ok_volume(sub { $result = $pool->clone_volume($voldstxml, $clone) }, 
"clone back to raw volume");
 
+diag "Cloning cloned volume back to raw format";
+my $voldstxml = $tck->generic_volume("tckdst", "raw", 
((1024*1024*50)+4096))->as_xml;
+my $result;
+ok_volume(sub { $result = $pool->clone_volume($voldstxml, $clone) }, 
"clone back to raw volume");
 
-$path = xpath($result, "string(/volume/target/path)");
 
-$st = stat($path);
-ok($st, "path $path exists");
+$path = xpath($result, "string(/volume/target/path)");
 
-is($st->size, ((1024*1024*50)+4096), "size is 50M");
+$st = stat($path);
+ok($st, "path $path exists");
 
-diag "Comparing data between source & result volume";
+is($st->size, ((1024*1024*50)+4096), "size is 50M");
 
-my $dstdigest = &digest($path);
+diag "Comparing data between source & result volume";
 
-is($srcdigest, $dstdigest, "digests match");
+my $dstdigest = &digest($path);
 
-lives_ok(sub { $clone->delete(0) }, "deleted clone volume");
-lives_ok(sub { $result->delete(0) }, "deleted result volume");
+is($srcdigest, $dstdigest, "digests match");
+
+lives_ok(sub { $clone->delete(0) }, "deleted clone volume");
+lives_ok(sub { $result->delete(0) }, "deleted result volume");
+}
 }
 
 
-- 
2.20.1

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


[libvirt] [tck PATCH 2/4] storage: fix/improve diagnostic messages

2019-02-07 Thread Laine Stump
Due to copy/paste, the tests for several other formats were described
as "qcow". Also, a couple of messages didn't give the image format.

Signed-off-by: Laine Stump 
---
 scripts/storage/100-create-vol-dir.t | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/storage/100-create-vol-dir.t 
b/scripts/storage/100-create-vol-dir.t
index 6262d69..9663d28 100644
--- a/scripts/storage/100-create-vol-dir.t
+++ b/scripts/storage/100-create-vol-dir.t
@@ -94,7 +94,7 @@ is($st->size, 1024*1024*50, "size is 50M");
 # overhead for a file
 ok($st->blocks >= (1024*1024*50/512), "alot of blocks allocated");
 
-lives_ok(sub { $vol->delete(0) }, "deleted volume");
+lives_ok(sub { $vol->delete(0) }, "deleted raw volume");
 
 
 
@@ -104,7 +104,7 @@ SKIP: {
 skip "qcow1 format not supported", 4;
 }
 
-ok_volume(sub { $vol = $pool->create_volume($volqcow1xml) }, "create qcow 
volume");
+ok_volume(sub { $vol = $pool->create_volume($volqcow1xml) }, "create qcow1 
volume");
 
 $path = xpath($vol, "string(/volume/target/path)");
 $st = stat($path);
@@ -121,7 +121,7 @@ SKIP: {
 
 
 
-ok_volume(sub { $vol = $pool->create_volume($volqcow2xml) }, "create qcow 
volume");
+ok_volume(sub { $vol = $pool->create_volume($volqcow2xml) }, "create qcow2 
volume");
 
 $path = xpath($vol, "string(/volume/target/path)");
 $st = stat($path);
@@ -132,12 +132,12 @@ ok($st, "path $path exists");
 # should be quite small :-)
 ok($st->size < 1024*1024, "basic qcow2 header is allocated");
 
-lives_ok(sub { $vol->delete(0) }, "deleted volume");
+lives_ok(sub { $vol->delete(0) }, "deleted qcow2 volume");
 
 
 
 
-ok_volume(sub { $vol = $pool->create_volume($volvmdkxml) }, "create qcow 
volume");
+ok_volume(sub { $vol = $pool->create_volume($volvmdkxml) }, "create vmdk 
volume");
 
 $path = xpath($vol, "string(/volume/target/path)");
 $st = stat($path);
@@ -148,12 +148,12 @@ ok($st, "path $path exists");
 # should be quite small :-)
 ok($st->size < 1024*1024, "basic vmdk header is allocated");
 
-lives_ok(sub { $vol->delete(0) }, "deleted volume");
+lives_ok(sub { $vol->delete(0) }, "deleted vmdk volume");
 
 
 
 
-ok_volume(sub { $vol = $pool->create_volume($volvpcxml) }, "create qcow 
volume");
+ok_volume(sub { $vol = $pool->create_volume($volvpcxml) }, "create vpc 
volume");
 
 $path = xpath($vol, "string(/volume/target/path)");
 $st = stat($path);
@@ -164,5 +164,5 @@ ok($st, "path $path exists");
 # should be quite small :-)
 ok($st->size < 1024*1024, "basic vpc header is allocated");
 
-lives_ok(sub { $vol->delete(0) }, "deleted volume");
+lives_ok(sub { $vol->delete(0) }, "deleted vpc volume");
 
-- 
2.20.1

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


[libvirt] [tck PATCH 0/4] A few libvirt-tck patches to fix false failures

2019-02-07 Thread Laine Stump
I found these when I ran the tck on RHEL8 beta.

Laine Stump (4):
  storage: skip qcow1 tests when qcow1 isn't supported by qemu-img
  storage: fix/improve diagnostic messages
  networks: remove stray use of brctl command
  nwfilter: allow for ebtables *not* removing leading 0 from mac
addresses

 scripts/networks/340-guest-network-bridge.t |  4 +-
 scripts/nwfilter/100-ping-still-working.t   |  4 +-
 scripts/nwfilter/210-no-mac-spoofing.t  |  4 +-
 scripts/storage/100-create-vol-dir.t| 36 +---
 scripts/storage/200-clone-vol-dir.t | 48 -
 5 files changed, 54 insertions(+), 42 deletions(-)

-- 
2.20.1

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


[libvirt] [python PATCH v4 2/2] Add virDomainCheckpoint APIs

2019-02-07 Thread Eric Blake
Copies heavily from existing virDomainSnapshot handling, regarding
what special cases the generator has to be taught and what overrides
need to be written.

Signed-off-by: Eric Blake 
---
 HACKING |  2 +
 MANIFEST.in |  1 +
 generator.py| 34 +++--
 libvirt-override-api.xml| 12 
 libvirt-override-virDomain.py   | 13 
 libvirt-override-virDomainCheckpoint.py | 19 +
 libvirt-override.c  | 96 +
 sanitytest.py   |  9 ++-
 typewrappers.c  | 13 
 typewrappers.h  | 10 +++
 10 files changed, 202 insertions(+), 7 deletions(-)
 create mode 100644 libvirt-override-virDomainCheckpoint.py

diff --git a/HACKING b/HACKING
index 6eeb9e6..39e7cd3 100644
--- a/HACKING
+++ b/HACKING
@@ -28,6 +28,8 @@ hand written source files
the virConnect class
  - libvirt-override-virDomain.py - high level overrides in
the virDomain class
+ - libvirt-override-virDomainCheckpoint.py - high level overrides in
+   the virDomainCheckpoint class
  - libvirt-override-virDomainSnapshot.py - high level overrides in
the virDomainSnapshot class
  - libvirt-override-virStoragePool.py - high level overrides in
diff --git a/MANIFEST.in b/MANIFEST.in
index b6788f4..5d2f559 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -23,6 +23,7 @@ include libvirt-override.c
 include libvirt-override.py
 include libvirt-override-virConnect.py
 include libvirt-override-virDomain.py
+include libvirt-override-virDomainCheckpoint.py
 include libvirt-override-virDomainSnapshot.py
 include libvirt-override-virStoragePool.py
 include libvirt-override-virStream.py
diff --git a/generator.py b/generator.py
index ffa3ce5..31535d4 100755
--- a/generator.py
+++ b/generator.py
@@ -35,6 +35,7 @@ libvirt_headers = [
 "libvirt",
 "libvirt-common",
 "libvirt-domain",
+"libvirt-domain-checkpoint",
 "libvirt-domain-snapshot",
 "libvirt-event",
 "libvirt-host",
@@ -364,6 +365,10 @@ py_types = {
 'virStream *':  ('O', "virStream", "virStreamPtr", "virStreamPtr"),
 'const virStream *':  ('O', "virStream", "virStreamPtr", "virStreamPtr"),

+'virDomainCheckpointPtr':  ('O', "virDomainCheckpoint", 
"virDomainCheckpointPtr", "virDomainCheckpointPtr"),
+'virDomainCheckpoint *':  ('O', "virDomainCheckpoint", 
"virDomainCheckpointPtr", "virDomainCheckpointPtr"),
+'const virDomainCheckpoint *':  ('O', "virDomainCheckpoint", 
"virDomainCheckpointPtr", "virDomainCheckpointPtr"),
+
 'virDomainSnapshotPtr':  ('O', "virDomainSnapshot", 
"virDomainSnapshotPtr", "virDomainSnapshotPtr"),
 'virDomainSnapshot *':  ('O', "virDomainSnapshot", "virDomainSnapshotPtr", 
"virDomainSnapshotPtr"),
 'const virDomainSnapshot *':  ('O', "virDomainSnapshot", 
"virDomainSnapshotPtr", "virDomainSnapshotPtr"),
@@ -536,6 +541,8 @@ skip_function = (
 'virSaveLastError', # We have our own python error wrapper
 'virFreeError', # Only needed if we use virSaveLastError
 'virConnectListAllDomains', # overridden in virConnect.py
+'virDomainListCheckpoints', # overridden in virDomain.py
+'virDomainCheckpointListChildren', # overridden in virDomainCheckpoint.py
 'virDomainListAllSnapshots', # overridden in virDomain.py
 'virDomainSnapshotListAllChildren', # overridden in virDomainSnapshot.py
 'virConnectListAllStoragePools', # overridden in virConnect.py
@@ -582,6 +589,7 @@ skip_function = (
 "virStoragePoolRef",
 "virStorageVolRef",
 "virStreamRef",
+"virDomainCheckpointRef",
 "virDomainSnapshotRef",

 # This functions shouldn't be called via the bindings (and even the docs
@@ -594,6 +602,8 @@ skip_function = (
 "virNWFilterGetConnect",
 "virStoragePoolGetConnect",
 "virStorageVolGetConnect",
+"virDomainCheckpointGetConnect",
+"virDomainCheckpointGetDomain",
 "virDomainSnapshotGetConnect",
 "virDomainSnapshotGetDomain",

@@ -1023,6 +1033,8 @@ classes_type = {
 "virStream *": ("._o", "virStream(self, _obj=%s)", "virStream"),
 "virConnectPtr": ("._o", "virConnect(_obj=%s)", "virConnect"),
 "virConnect *": ("._o", "virConnect(_obj=%s)", "virConnect"),
+"virDomainCheckpointPtr": ("._o", "virDomainCheckpoint(self,_obj=%s)", 
"virDomainCheckpoint"),
+"virDomainCheckpoint *": ("._o", "virDomainCheckpoint(self, _obj=%s)", 
"virDomainCheckpoint"),
 "virDomainSnapshotPtr": ("._o", "virDomainSnapshot(self,_obj=%s)", 
"virDomainSnapshot"),
 "virDomainSnapshot *": ("._o", "virDomainSnapshot(self, _obj=%s)", 
"virDomainSnapshot"),
 }
@@ -1031,7 +1043,7 @@ primary_classes = ["virDomain", "virNetwork", 
"virInterface",
"virStoragePool", "virStorageVol",
"virConnect", "virNodeDevice", "virSecret",
"virNWFilter", "virNWFilterBinding",
-   "virStream", 

[libvirt] [python PATCH v4 1/2] generator.py: typo fix

2019-02-07 Thread Eric Blake
Signed-off-by: Eric Blake 
---
 generator.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/generator.py b/generator.py
index 6cb923f..ffa3ce5 100755
--- a/generator.py
+++ b/generator.py
@@ -1095,7 +1095,7 @@ def is_python_noninteger_type (name):
 return name[-1:] == "*"

 def nameFixup(name, classe, type, file):
-# avoid a desastrous clash
+# avoid a disastrous clash
 listname = classe + "List"
 ll = len(listname)
 l = len(classe)
-- 
2.20.1

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


[libvirt] [python PATCH v4 0/2] Python bindings for virDomainCheckpoint

2019-02-07 Thread Eric Blake
Python counterpart to:
https://www.redhat.com/archives/libvir-list/2019-February/msg00252.html

I'll push patch 1 now as a trivial fix; patch 2 can't go in until
the corresponding libvirt patches are accepted (which may in turn
necessitate minor tweaks if I make any API changes there based on
the review). But with this patch, it should be possible to test
python bindings to all the proposed C changes.

Eric Blake (2):
  generator.py: typo fix
  Add virDomainCheckpoint APIs

 HACKING |  2 +
 MANIFEST.in |  1 +
 generator.py| 36 --
 libvirt-override-api.xml| 12 
 libvirt-override-virDomain.py   | 13 
 libvirt-override-virDomainCheckpoint.py | 19 +
 libvirt-override.c  | 96 +
 sanitytest.py   |  9 ++-
 typewrappers.c  | 13 
 typewrappers.h  | 10 +++
 10 files changed, 203 insertions(+), 8 deletions(-)
 create mode 100644 libvirt-override-virDomainCheckpoint.py

-- 
2.20.1

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


Re: [libvirt] [PATCH v2 32/32] qemu: caps: Add lockout for -blockdev if QEMU_CAPS_SCSI_DISK_DEVICE_ID is not present

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:47:05PM +0100, Peter Krempa wrote:

Avoid regressions by disallowing the BLOCKDEV capability.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c | 5 +
1 file changed, 5 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 31/32] qemu: Use the 'device_id' property of SCSI disks to avoid regressing

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:47:04PM +0100, Peter Krempa wrote:

QEMU accidentally exposed the id of -drive (or same value as disk
serial, if provided) in one of the identifiers visible from the guest.

To avoid regression in case when -blockdev will be used we need to
always specify it ourselves.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_command.c   | 22 +++
.../controller-virtio-scsi.x86_64-latest.args | 20 -
.../disk-cache.x86_64-latest.args |  4 ++--
.../disk-scsi-device-auto.x86_64-latest.args  |  3 ++-
.../disk-scsi.x86_64-latest.args  | 16 --
.../disk-shared.x86_64-latest.args|  5 +++--
...threads-virtio-scsi-pci.x86_64-latest.args |  4 ++--
7 files changed, 50 insertions(+), 24 deletions(-)

@@ -2021,6 +2037,12 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
  disk->info.addr.drive.target,
  disk->info.addr.drive.unit);
}
+
+if (scsiVPDDeviceId) {
+virBufferAddLit(&opt, ",device_id=");
+virBufferEscape(&opt, '\\', " ", "%s", scsiVPDDeviceId);


commas should be escaped, not spaces


+}
+
break;

case VIR_DOMAIN_DISK_BUS_SATA:


Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread John Ferlan



On 2/7/19 11:54 AM, Andrea Bolognani wrote:
> On Thu, 2019-02-07 at 17:36 +0100, Erik Skultety wrote:
>> On Thu, Feb 07, 2019 at 05:11:24PM +0100, Andrea Bolognani wrote:
>>> On Thu, 2019-02-07 at 16:55 +0100, Erik Skultety wrote:
 On Thu, Feb 07, 2019 at 04:24:13PM +0100, Andrea Bolognani wrote:
> Please keep the semicolon! If a macro is used like a function, then
> its call sites should also look like those of a function.

 Except VIR_DEFINE_AUTOPTR_FUNC macro is not used like a function, it 
 defines a
 function.
>>>
>>> Sure, and the way you make that happen is by writing
>>>
>>>   MACRO_NAME(argument_one, argument_two);
>>>
>>> How is that not using it like a function? :)
>>
>> I may need to replace my dictionary, because the way I understand the
>> expression "like a function" is that the macro is called like function and it
>> behaves like a function, i.e. returns a value, IOW by using the macro its
>> expansion will perform the usual set of operation on the stack that a 
>> function
>> call involves (push parameters, return address, jump into function,
>> pop the return value...)
> 
> I guess abort() is not a function either then, since it doesn't have
> any parameters to push or values to return! :P
> 
> Anyway, the point is that we have already started mandating the use
> of semicolon after other macros that expand to definitions, such as
> VIR_ENUM_DECL(), VIR_ENUM_IMPL(), and VIR_ONCE_GLOBAL_INIT(): we
> should do the same for VIR_DEFINE_AUTOPTR_FUNC() and increase
> consistency instead of pushing in the opposite direction.
> 

Since the issue is consistency, how about a patch that adds the ; to the
existing VIR_DEFINE_AUTOPTR_FUNC's that don't have it?  Ironically, I
found one that has it:

VIR_DEFINE_AUTOPTR_FUNC(virSEVCapability, virSEVCapabilitiesFree);

John

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


Re: [libvirt] [PATCH v2 30/32] qemu: caps: Introduce capability for 'device_id' property of 'scsi-disk'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:47:03PM +0100, Peter Krempa wrote:

The property allows to control the guest-visible content of the vendor
specific designator of the 'Device Identification' page of a SCSI
device's VPD (vital product data).

QEMU was leaking the id string of -drive as the value if the 'serial' of
the disk was not specified. Switching to -blockdev would impose an ABI
change.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 +
3 files changed, 4 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 29/32] test: qemucaps: Update caps with scsi 'device_id' property

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:47:02PM +0100, Peter Krempa wrote:

Based on qemu commit 'v3.1.0-1445-ga61faa3d02'. Will allow checking
for the scsi 'device_id' property.

Signed-off-by: Peter Krempa 
---
.../caps_4.0.0.x86_64.replies | 2296 +
.../caps_4.0.0.x86_64.xml |   29 +-
2 files changed, 1248 insertions(+), 1077 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 27/32] qemu: command: Drop formatting of 'media=cdrom' from -drive

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:47:00PM +0100, Peter Krempa wrote:

For SCSI, IDE, and AHCI cdroms the appropriate device types which select
the correct media are used. In qemu there's one other code path that
looks at -drive media=cdrom in the XEN pv code. Thankfully we don't
support it with qemu (see qemuBuildDiskDeviceStr). All other devices
ignore it as the comment states, thus we can drop that code.

The test fallout is expectedly only in the test added for uncommon cdrom
types.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_command.c  | 9 -
.../disk-cdrom-bus-other.x86_64-latest.args  | 9 -
2 files changed, 4 insertions(+), 14 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 28/32] tests: qemuxml2argv: Add a 'serial' value for a SCSI disk

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:47:01PM +0100, Peter Krempa wrote:

Upcoming addition of a new field will need to make sure that SCSI disk
serial is tested as well. Add a case to one of the existing tests.

Signed-off-by: Peter Krempa 
---
tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/disk-scsi.xml| 1 +
tests/qemuxml2xmloutdata/disk-scsi.xml  | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 26/32] qemu: Forbid cdroms on virtio bus

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:59PM +0100, Peter Krempa wrote:

Attempting to create an empty virtio-blk drive results into:
-device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0xc,drive=drive-virtio-disk1,id=virtio-disk1:
 Device needs media, but drive is empty

Attempting to eject media from virtio-blk based drive results into:
error: internal error: unable to execute QEMU command 'eject': Device 
'drive-virtio-disk0' is not removable

Forbid configurations where users would attempt to use cdroms in virtio
bus.

Test fallout apart from the recently added case contains one more wrong
example which is not really relevant to the tested code, thus I've
changed the type.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_domain.c  |  8 
.../disk-cdrom-bus-other.x86_64-latest.args |  7 ---
tests/qemuxml2argvdata/disk-cdrom-bus-other.xml | 11 ---
.../disk-scsi-disk-vpd-build-error.xml  |  2 +-
tests/qemuxml2argvdata/pci-autofill-addr.args   |  4 ++--
tests/qemuxml2argvdata/pci-autofill-addr.xml|  2 +-
tests/qemuxml2xmloutdata/disk-cdrom-bus-other.xml   | 13 -
tests/qemuxml2xmloutdata/pci-autofill-addr.xml  |  2 +-
8 files changed, 13 insertions(+), 36 deletions(-)




diff --git a/tests/qemuxml2argvdata/disk-cdrom-bus-other.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-cdrom-bus-other.x86_64-latest.args
index 9dd45133fc..a64da3edd6 100644
--- a/tests/qemuxml2argvdata/disk-cdrom-bus-other.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-cdrom-bus-other.x86_64-latest.args
@@ -31,13 +31,6 @@ removable=off \
-drive if=none,id=drive-usb-disk1,media=cdrom,readonly=on \
-device usb-storage,bus=usb.0,port=2,drive=drive-usb-disk1,id=usb-disk1,\
removable=off \
--drive file=/root/boot1.iso,format=raw,if=none,id=drive-virtio-disk0,\
-media=cdrom,readonly=on,cache=none \
--device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=drive-virtio-disk0,\
-id=virtio-disk0,write-cache=on \
--drive if=none,id=drive-virtio-disk1,media=cdrom,readonly=on \
--device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk1,\
-id=virtio-disk1,write-cache=on \
-drive file=/root/boot2.iso,format=raw,if=sd,index=2,media=cdrom,readonly=on \
-drive if=sd,index=3,media=cdrom,readonly=on \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
diff --git a/tests/qemuxml2argvdata/disk-cdrom-bus-other.xml 
b/tests/qemuxml2argvdata/disk-cdrom-bus-other.xml
index 2ed86b0900..a142373afb 100644
--- a/tests/qemuxml2argvdata/disk-cdrom-bus-other.xml
+++ b/tests/qemuxml2argvdata/disk-cdrom-bus-other.xml
@@ -26,17 +26,6 @@
  
  

-
-  
-  
-  
-  
-
-
-  
-  
-  
-

  
  


Consider squashing these hunks into the previous patch.

Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 25/32] tests: qemuxml2argv: Add CDROM disks for all untested buses

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:58PM +0100, Peter Krempa wrote:

Add full and empty cdroms on 'usb', 'virtio', and 'sd' bus to have test


Feels odd to add 'virtio' here if you remove it in the next patch.


coverage. Note that this does not guarantee that qemu will accept them.

Signed-off-by: Peter Krempa 
---
.../disk-cdrom-bus-other.x86_64-latest.args   | 45 +
.../qemuxml2argvdata/disk-cdrom-bus-other.xml | 58 +
tests/qemuxml2argvtest.c  |  1 +
.../disk-cdrom-bus-other.xml  | 64 +++
tests/qemuxml2xmltest.c   |  1 +
5 files changed, 169 insertions(+)
create mode 100644 
tests/qemuxml2argvdata/disk-cdrom-bus-other.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/disk-cdrom-bus-other.xml
create mode 100644 tests/qemuxml2xmloutdata/disk-cdrom-bus-other.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread Andrea Bolognani
On Thu, 2019-02-07 at 17:36 +0100, Erik Skultety wrote:
> On Thu, Feb 07, 2019 at 05:11:24PM +0100, Andrea Bolognani wrote:
> > On Thu, 2019-02-07 at 16:55 +0100, Erik Skultety wrote:
> > > On Thu, Feb 07, 2019 at 04:24:13PM +0100, Andrea Bolognani wrote:
> > > > Please keep the semicolon! If a macro is used like a function, then
> > > > its call sites should also look like those of a function.
> > > 
> > > Except VIR_DEFINE_AUTOPTR_FUNC macro is not used like a function, it 
> > > defines a
> > > function.
> > 
> > Sure, and the way you make that happen is by writing
> > 
> >   MACRO_NAME(argument_one, argument_two);
> > 
> > How is that not using it like a function? :)
> 
> I may need to replace my dictionary, because the way I understand the
> expression "like a function" is that the macro is called like function and it
> behaves like a function, i.e. returns a value, IOW by using the macro its
> expansion will perform the usual set of operation on the stack that a function
> call involves (push parameters, return address, jump into function,
> pop the return value...)

I guess abort() is not a function either then, since it doesn't have
any parameters to push or values to return! :P

Anyway, the point is that we have already started mandating the use
of semicolon after other macros that expand to definitions, such as
VIR_ENUM_DECL(), VIR_ENUM_IMPL(), and VIR_ONCE_GLOBAL_INIT(): we
should do the same for VIR_DEFINE_AUTOPTR_FUNC() and increase
consistency instead of pushing in the opposite direction.

-- 
Andrea Bolognani / Red Hat / Virtualization

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


Re: [libvirt] [PATCH v2 24/32] qemu: command: Use correct type for switch in qemuBuildDiskDeviceStr

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:57PM +0100, Peter Krempa wrote:

Cast disk->bus to proper type and add missing values to the enum so it's
more obvious what types are supported.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_command.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 10/10] qemu: caps: Don't call 'query-events' when we probe them from QMP schema

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:14PM +0100, Peter Krempa wrote:

Avoid calling the command and fix test fallout.



Also, there's a public bug you can reference here:
https://bugzilla.redhat.com/show_bug.cgi?id=1673320

Jano


Note that for clarity and size this does not include the fix to the
numbering of commands. I've used the following command to fix them and
it makes tests pass:

for i in tests/qemucapabilitiesdata/*.replies; do ./tests/qemucapsfixreplies 
$i; done
---


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

Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread Erik Skultety
On Thu, Feb 07, 2019 at 05:11:24PM +0100, Andrea Bolognani wrote:
> On Thu, 2019-02-07 at 16:55 +0100, Erik Skultety wrote:
> > On Thu, Feb 07, 2019 at 04:24:13PM +0100, Andrea Bolognani wrote:
> > > Please keep the semicolon! If a macro is used like a function, then
> > > its call sites should also look like those of a function.
> >
> > Except VIR_DEFINE_AUTOPTR_FUNC macro is not used like a function, it 
> > defines a
> > function.
>
> Sure, and the way you make that happen is by writing
>
>   MACRO_NAME(argument_one, argument_two);
>
> How is that not using it like a function? :)

I may need to replace my dictionary, because the way I understand the
expression "like a function" is that the macro is called like function and it
behaves like a function, i.e. returns a value, IOW by using the macro its
expansion will perform the usual set of operation on the stack that a function
call involves (push parameters, return address, jump into function,
pop the return value...)

Erik

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


Re: [libvirt] [PATCH 10/10] qemu: caps: Don't call 'query-events' when we probe them from QMP schema

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:14PM +0100, Peter Krempa wrote:

Avoid calling the command and fix test fallout.

Note that for clarity and size this does not include the fix to the
numbering of commands. I've used the following command to fix them and
it makes tests pass:

for i in tests/qemucapabilitiesdata/*.replies; do ./tests/qemucapsfixreplies 
$i; done
---
src/qemu/qemu_capabilities.c  |  10 +-
.../caps_2.10.0.aarch64.replies   | 122 
.../caps_2.10.0.ppc64.replies | 122 
.../caps_2.10.0.s390x.replies | 122 
.../caps_2.10.0.x86_64.replies| 122 
.../caps_2.11.0.s390x.replies | 122 
.../caps_2.11.0.x86_64.replies| 122 
.../caps_2.12.0.aarch64.replies   | 128 
.../caps_2.12.0.ppc64.replies | 128 
.../caps_2.12.0.s390x.replies | 128 
.../caps_2.12.0.x86_64.replies| 128 
.../caps_2.5.0.x86_64.replies | 116 ---
.../caps_2.6.0.aarch64.replies| 122 
.../caps_2.6.0.ppc64.replies  | 122 
.../caps_2.6.0.x86_64.replies | 122 
.../caps_2.7.0.s390x.replies  | 122 
.../caps_2.7.0.x86_64.replies | 122 
.../caps_2.8.0.s390x.replies  | 122 
.../caps_2.8.0.x86_64.replies | 122 
.../caps_2.9.0.ppc64.replies  | 122 
.../caps_2.9.0.s390x.replies  | 122 
.../caps_2.9.0.x86_64.replies | 122 
.../caps_3.0.0.ppc64.replies  | 131 -
.../caps_3.0.0.riscv32.replies| 134 -
.../caps_3.0.0.riscv64.replies| 134 -
.../caps_3.0.0.s390x.replies  | 134 -
.../caps_3.0.0.x86_64.replies | 134 -
.../caps_3.1.0.ppc64.replies  | 137 --
.../caps_3.1.0.x86_64.replies | 137 --
.../caps_4.0.0.riscv32.replies| 137 --
.../caps_4.0.0.riscv64.replies| 137 --
.../caps_4.0.0.x86_64.replies | 137 --
32 files changed, 4 insertions(+), 3938 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 09/10] qemu: caps: Probe events from 'query-qmp-schema' rather than 'query-events'

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:13PM +0100, Peter Krempa wrote:

QEMU plans to deprecate 'query-events' as it's non-extensible. Events
are also described by 'query-qmp-schema' so we can use that one instead.

This patch adds detection of events to
virQEMUCapsProbeQMPSchemaCapabilities using the same structure declaring
them for the old approach (virQEMUCapsEvents). This is possible as the
name is the same in the QMP schema and our detector supports that
trivially.

For any complex queries virQEMUCapsQMPSchemaQueries can be used in the
future.

For now we still call 'query-events' and discard the result so that it's
obvious that the tests pass. This will be cleaned up later.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c | 14 ++
1 file changed, 14 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 08/10] qemu: caps: Always assume QEMU_CAPS_DEVICE_TRAY_MOVED

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:12PM +0100, Peter Krempa wrote:

The event was added by qemu commit 6f382ed226f3 released in v1.1.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c   | 1 -
src/qemu/qemu_capabilities.h   | 2 +-
src/qemu/qemu_hotplug.c| 5 ++---
tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  | 1 -
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 -
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 -
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml  | 1 -
tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml| 1 -
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml| 1 -
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml| 1 -
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml| 1 -
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml| 1 -
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 -
tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 -
tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 -
tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 -
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 -
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 -
39 files changed, 3 insertions(+), 41 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 07/10] qemu: caps: Always assume QEMU_CAPS_DEVICE_DEL_EVENT

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:11PM +0100, Peter Krempa wrote:

DEVICE_DELETED was added in qemu commit 0402a5d65ec00 which was released
in v1.5.0.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c   |  1 -
src/qemu/qemu_capabilities.h   |  2 +-
src/qemu/qemu_domain.c |  3 ---
src/qemu/qemu_hotplug.c| 10 +-
src/qemu/qemu_process.c|  3 ---
tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml |  1 -
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  |  1 -
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  |  1 -
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |  1 -
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  1 -
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml  |  1 -
tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml|  1 -
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml|  1 -
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml|  1 -
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml|  1 -
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml|  1 -
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml|  1 -
tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  |  1 -
tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  |  1 -
tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml|  1 -
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml|  1 -
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   |  1 -
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  |  1 -
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  |  1 -
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   |  1 -
tests/qemuhotplugtest.c|  1 -
42 files changed, 2 insertions(+), 54 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 06/10] tests: qemuhotplug: Remove leftovers for non-event testing

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:10PM +0100, Peter Krempa wrote:

DO_TEST_ATTACH and DO_TEST_ATTACH_EVENT now do the same thing so we can
remove the latter including the infrastructure.



Bye, leftovers!


Signed-off-by: Peter Krempa 
---
tests/qemuhotplugtest.c | 56 ++---
1 file changed, 25 insertions(+), 31 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 05/10] tests: qemuhotplug: Use DEVICE_DELETED event in all hotunplug tests

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:09PM +0100, Peter Krempa wrote:

Currently all supported qemu versions now have support for the
DEVICE_DELETED event. This means that testing the old approach is a
waste of time.

Always add the QEMU_CAPS_DEVICE_DEL_EVENT capability in the hotplug test
and fix existing test cases.

The 'disk-virtio', 'disk-usb', 'disk-scsi', and 'disk-scsi-2' already
had variants that used the event, so the non-event variants will be
removed.

For all other cases the QMP_DEVICE_DELETED macro is used to add the
correct reply.

Signed-off-by: Peter Krempa 
---
tests/qemuhotplugtest.c | 59 +
1 file changed, 12 insertions(+), 47 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [RFC PATCH] udev: Remove udev handle from main loop when udev thread stops

2019-02-07 Thread Marc Hartmayer
On Thu, Feb 07, 2019 at 05:08 PM +0100, Marc Hartmayer  
wrote:
> Commit "nodedev: Move device enumumeration out of nodeStateInitialize"
> (9f0ae0b18e3e620) has moved the heavy task of device enumeration into
> a separate thread. The problem with this commit is that there is a
> functionality change in the cleanup when udevEnumerateDevices
> fails. Before commit 9f0ae0b18e3e620, the entire driver was cleaned up
> by calling nodeStateCleanup (defined in node_device_udev.c) which
> resulted in libvirtd stopping with the error message
> 'daemonRunStateInit:800 : Driver state initialization failed'. With
> the commit 9f0ae0b18e3e620 there is only a signal to the udev thread
> that it must stop. This means that for example the watch handle isn't
> removed from the main loop and this can result in the main loop
> consuming 100% CPU time as soon as a new udev event occurs.
>
> This patch proposes a simple solution to the described problem. In
> case the udev thread stops the watch handle is removed from the main
> loop.
>
> Fixes: 9f0ae0b18e3e620 ("nodedev: Move device enumumeration out of 
> nodeStateInitialize")
> Signed-off-by: Marc Hartmayer 
> ---
>
> Note: I'm not sure whether we should stop libvirtd (as it would have
>   been done before) or if this patch is already sufficient.
>
> ---
>  src/node_device/node_device_udev.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/node_device/node_device_udev.c 
> b/src/node_device/node_device_udev.c
> index b1e5f00067e8..299f55260129 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1628,6 +1628,13 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED)
>  }
>
>  if (priv->threadQuit) {
> +if (priv->watch != -1) {
> +/* Since the udev thread getting stopped remove the
> + * watch handle from the main loop */
> +virEventRemoveHandle(priv->watch);
> +priv->watch = -1;
> +}
> +
>  virObjectUnlock(priv);
>  return;
>  }
> --
> 2.17.0
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Sorry… I forgot to add John to CC.

Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


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

Re: [libvirt] [PATCH 04/10] tests: qemuhotplug: Remove unused test macro DO_TEST_DETACH_EVENT

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:08PM +0100, Peter Krempa wrote:

This variant is unused as we create the object including capabilities
with DO_TEST_ATTACH_EVENT, which is then reused.

Signed-off-by: Peter Krempa 
---
tests/qemuhotplugtest.c | 3 ---
1 file changed, 3 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [RFC PATCH] udev: Remove udev handle from main loop when udev thread stops

2019-02-07 Thread Marc Hartmayer
On Thu, Feb 07, 2019 at 05:08 PM +0100, Marc Hartmayer  
wrote:
> Commit "nodedev: Move device enumumeration out of nodeStateInitialize"
> (9f0ae0b18e3e620) has moved the heavy task of device enumeration into
> a separate thread. The problem with this commit is that there is a
> functionality change in the cleanup when udevEnumerateDevices
> fails. Before commit 9f0ae0b18e3e620, the entire driver was cleaned up
> by calling nodeStateCleanup (defined in node_device_udev.c) which
> resulted in libvirtd stopping with the error message
> 'daemonRunStateInit:800 : Driver state initialization failed'. With
> the commit 9f0ae0b18e3e620 there is only a signal to the udev thread
> that it must stop. This means that for example the watch handle isn't
> removed from the main loop and this can result in the main loop
> consuming 100% CPU time as soon as a new udev event occurs.
>
> This patch proposes a simple solution to the described problem. In
> case the udev thread stops the watch handle is removed from the main
> loop.

Another option would be to stop libvirtd if udevEnumerateDevices fails
in nodeStateInitializeEnumerate.

[…snip]


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

Re: [libvirt] [PATCH 03/10] qemu: caps: Always assume QEMU_CAPS_SEAMLESS_MIGRATION

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:07PM +0100, Peter Krempa wrote:

The event was added by qemu commit 2fdd16e239c2a2 released in v1.3.0.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c   |  1 -
src/qemu/qemu_capabilities.h   |  2 +-
src/qemu/qemu_command.c| 10 +++---
src/qemu/qemu_migration.c  |  3 +--
tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml   |  1 -

[...]

tests/qemuxml2argvdata/graphics-spice.args |  2 +-
tests/qemuxml2argvdata/name-escape.args|  2 +-
tests/qemuxml2argvdata/q35-virt-manager-basic.args |  2 +-
tests/qemuxml2argvdata/serial-spiceport.args   |  3 ++-
tests/qemuxml2argvdata/video-virtio-gpu-spice-gl.args  |  2 +-
59 files changed, 30 insertions(+), 66 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v1 1/1] qemu_process.c: add CAP_IPC_LOCK when using libcap-ng

2019-02-07 Thread Daniel Henrique Barboza
Alexey replied in the QEMU mailing list in his patch series. Here's the 
link:


https://patchwork.kernel.org/cover/10767473/

Based on Alexey's explanation, I was able to create a quick PoC that changes
the calculation of the passthrough limit memory for the PPC64 machine in
qemu_domain.c, qemuDomainGetMemLockLimitBytes, to make the guest
boot up without IPC_LOCK privileges and with NVLink2 passthrough. Given
that this is based on how the NVLink2 support is implemented in the pseries
machine in QEMU, this code will need to verify the GPU model being
passed-through as well.

This deprecates the solution I was aiming for in this patch. There's no need
to grant IPC_LOCK for guests that uses VFIO because the guest will not
allocate RAM beyond the limit. This new approach is also confined in the 
PPC64

code, thus the impact is minimal to x86 and other archs*.


Thanks,


DHB



* we can always generalize this code if other archs ends up doing DMA 
resize with

passthrough devices the same way Power is doing.


On 2/6/19 3:05 PM, Daniel Henrique Barboza wrote:



On 2/6/19 2:34 PM, Alex Williamson wrote:

[cc +Alexey]

On Wed,  6 Feb 2019 13:44:53 -0200
Daniel Henrique Barboza  wrote:


QEMU virtual machines with PCI passthrough of certain devices,
such as the NVIDIA Tesla V100 GPU, requires allocation of an
amount of memory pages that can break KVM limits. When that
happens, the KVM module checks if the process is IPC_LOCK capable
and, in case it doesn't, it refuses to allocate the mem pages,
causing the guest to misbehave.

When Libvirt is compiled to use libcap-ng support, the resulting
QEMU guest Libvirt creates does not have CAP_IPC_LOCK, hurting
guests that are doing PCI passthrough and that requires extra
memory to work.

This patch addresses this issue by checking whether we're
running with libcap-ng support (WITH_CAPNG) and if the
guest is using PCI passthrough with the VFIO driver. If those
conditions are met, allow CAP_IPC_LOCK capability to the
QEMU process.

That effectively allows the QEMU process to lock all the memory it
wants and thereby generate a denial of service on the host, which is
the thing we're originally trying to prevent by accounting pinned pages
and enforcing the user's locked memory limits.  The commit log doesn't
mention that this is only seen on POWER, but I infer that from the
previous thread.  Does this happen for any assigned device on POWER or
is this something to do with the new NVLink support that Alexey has been
adding?  Thanks,


You're right in both accounts. The behavior I am addressing here is 
seen in a

Power 9 server using a QEMU build with NVLink2 passthrough support
from Alexey.

Not all assigned devices in Power wants to lock more memory than KVM 
allows,

at least as far as I can tell.

About your argument of allowing QEMU to lock all memory, I agree that
is a problem. Aside from trusting that QEMU will not get greedy and lock
down all the host memory (which is what I'm doing here, in a sense), I
can only speculate alternatives. I'd rather wait for Alexey's input.



Thanks,


DHB






Alex


Signed-off-by: Daniel Henrique Barboza 
---
  src/libvirt_private.syms |  1 +
  src/qemu/qemu_process.c  | 34 ++
  src/util/virhostdev.c    |  2 +-
  src/util/virhostdev.h    |  4 
  4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6b401aa5e0..b7384a67a8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1992,6 +1992,7 @@ virHostCPUStatsAssign;
    # util/virhostdev.h
  virHostdevFindUSBDevice;
+virHostdevGetPCIHostDeviceList;
  virHostdevIsMdevDevice;
  virHostdevIsSCSIDevice;
  virHostdevManagerGetDefault;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0583eb03f2..24aef5904f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4997,6 +4997,38 @@ qemuProcessSetupRawIO(virQEMUDriverPtr driver,
  }
    +static void
+qemuProcessSetupVFIOCaps(virDomainObjPtr vm ATTRIBUTE_UNUSED,
+ virCommandPtr cmd ATTRIBUTE_UNUSED)
+{
+#if WITH_CAPNG
+    virPCIDeviceListPtr pcidevs = NULL;
+    bool has_vfio = false;
+    size_t i;
+
+    pcidevs = virHostdevGetPCIHostDeviceList(vm->def->hostdevs,
+ vm->def->nhostdevs);
+    if (!pcidevs)
+    return;
+
+    for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
+    virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
+    if (virPCIDeviceGetStubDriver(pci) == 
VIR_PCI_STUB_DRIVER_VFIO) {

+    has_vfio = true;
+    break;
+    }
+    }
+
+    if (has_vfio) {
+    VIR_DEBUG("Adding CAP_IPC_LOCK to QEMU process");
+    virCommandAllowCap(cmd, CAP_IPC_LOCK);
+    }
+
+    virObjectUnref(pcidevs);
+#endif
+}
+
+
  static int
  qemuProcessSetupBalloon(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
@@ -6569,6 +6601,8 @@ qemuProcessLaunch(virConnectPtr conn,
  if (qemuProcessSetupRawIO(driver, vm

Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread Andrea Bolognani
On Thu, 2019-02-07 at 16:55 +0100, Erik Skultety wrote:
> On Thu, Feb 07, 2019 at 04:24:13PM +0100, Andrea Bolognani wrote:
> > Please keep the semicolon! If a macro is used like a function, then
> > its call sites should also look like those of a function.
> 
> Except VIR_DEFINE_AUTOPTR_FUNC macro is not used like a function, it defines a
> function.

Sure, and the way you make that happen is by writing

  MACRO_NAME(argument_one, argument_two);

How is that not using it like a function? :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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


Re: [libvirt] [PATCH 02/10] qemu: Clean up usage of qemuDomainUpdateCurrentMemorySize

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:06PM +0100, Peter Krempa wrote:

Remove the uneeded attribute and return value.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_domain.c | 15 ---
src/qemu/qemu_domain.h |  3 +--
src/qemu/qemu_driver.c |  7 ++-
3 files changed, 7 insertions(+), 18 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

[libvirt] [RFC PATCH] udev: Remove udev handle from main loop when udev thread stops

2019-02-07 Thread Marc Hartmayer
Commit "nodedev: Move device enumumeration out of nodeStateInitialize"
(9f0ae0b18e3e620) has moved the heavy task of device enumeration into
a separate thread. The problem with this commit is that there is a
functionality change in the cleanup when udevEnumerateDevices
fails. Before commit 9f0ae0b18e3e620, the entire driver was cleaned up
by calling nodeStateCleanup (defined in node_device_udev.c) which
resulted in libvirtd stopping with the error message
'daemonRunStateInit:800 : Driver state initialization failed'. With
the commit 9f0ae0b18e3e620 there is only a signal to the udev thread
that it must stop. This means that for example the watch handle isn't
removed from the main loop and this can result in the main loop
consuming 100% CPU time as soon as a new udev event occurs.

This patch proposes a simple solution to the described problem. In
case the udev thread stops the watch handle is removed from the main
loop.

Fixes: 9f0ae0b18e3e620 ("nodedev: Move device enumumeration out of 
nodeStateInitialize")
Signed-off-by: Marc Hartmayer 
---

Note: I'm not sure whether we should stop libvirtd (as it would have
  been done before) or if this patch is already sufficient.

---
 src/node_device/node_device_udev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index b1e5f00067e8..299f55260129 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1628,6 +1628,13 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED)
 }
 
 if (priv->threadQuit) {
+if (priv->watch != -1) {
+/* Since the udev thread getting stopped remove the
+ * watch handle from the main loop */
+virEventRemoveHandle(priv->watch);
+priv->watch = -1;
+}
+
 virObjectUnlock(priv);
 return;
 }
-- 
2.17.0

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


Re: [libvirt] [PATCH 01/10] qemu: caps: Always assume QEMU_CAPS_BALLOON_EVENT

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 01:28:05PM +0100, Peter Krempa wrote:

The event was added to qemu by commit 973603a813c5d60 which is contained
in the 1.2.0 release.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c  |  1 -
src/qemu/qemu_capabilities.h  |  2 +-
src/qemu/qemu_domain.c| 43 ++-
src/qemu/qemu_driver.c| 16 +++
.../caps_1.5.3.x86_64.xml |  1 -
.../caps_1.6.0.x86_64.xml |  1 -
.../caps_1.7.0.x86_64.xml |  1 -
.../caps_2.1.1.x86_64.xml |  1 -
.../caps_2.10.0.aarch64.xml   |  1 -
.../caps_2.10.0.ppc64.xml |  1 -
.../caps_2.10.0.s390x.xml |  1 -
.../caps_2.10.0.x86_64.xml|  1 -
.../caps_2.11.0.s390x.xml |  1 -
.../caps_2.11.0.x86_64.xml|  1 -
.../caps_2.12.0.aarch64.xml   |  1 -
.../caps_2.12.0.ppc64.xml |  1 -
.../caps_2.12.0.s390x.xml |  1 -
.../caps_2.12.0.x86_64.xml|  1 -
.../caps_2.4.0.x86_64.xml |  1 -
.../caps_2.5.0.x86_64.xml |  1 -
.../caps_2.6.0.aarch64.xml|  1 -
.../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  1 -
.../caps_2.6.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_2.7.0.s390x.xml |  1 -
.../caps_2.7.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_2.8.0.s390x.xml |  1 -
.../caps_2.8.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  1 -
.../qemucapabilitiesdata/caps_2.9.0.s390x.xml |  1 -
.../caps_2.9.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 -
.../caps_3.0.0.riscv32.xml|  1 -
.../caps_3.0.0.riscv64.xml|  1 -
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  1 -
.../caps_3.0.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 -
.../caps_3.1.0.x86_64.xml |  1 -
.../caps_4.0.0.riscv32.xml|  1 -
.../caps_4.0.0.riscv64.xml|  1 -
.../caps_4.0.0.x86_64.xml |  1 -
40 files changed, 11 insertions(+), 87 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 0/3] bhyve: implement MSRs ignore unknown writes feature

2019-02-07 Thread Roman Bogorodskiy
  Daniel P. Berrangé wrote:

> On Thu, Feb 07, 2019 at 06:05:25PM +0400, Roman Bogorodskiy wrote:
> >   Daniel P. Berrangé wrote:
> > 
> > > On Wed, Feb 06, 2019 at 04:00:06PM -0500, Cole Robinson wrote:
> > > > On 1/25/19 12:54 PM, Roman Bogorodskiy wrote:
> > > > > 
> > > > > Roman Bogorodskiy (3):
> > > > >   conf: introduce 'msrs' feature
> > > > >   bhyve: implement MSRs ignore unknown writes feature
> > > > >   news: document bhyve msrs feature
> > > > > 
> > > > >  docs/drvbhyve.html.in | 16 +
> > > > >  docs/formatdomain.html.in |  1 +
> > > > >  docs/news.xml | 11 ++
> > > > >  docs/schemas/domaincommon.rng | 14 
> > > > >  src/bhyve/bhyve_command.c |  4 +++
> > > > >  src/conf/domain_conf.c| 33 +
> > > > >  src/conf/domain_conf.h|  8 +
> > > > >  src/qemu/qemu_domain.c|  1 +
> > > > >  .../bhyvexml2argvdata/bhyvexml2argv-msrs.args | 10 ++
> > > > >  .../bhyvexml2argv-msrs.ldargs |  3 ++
> > > > >  .../bhyvexml2argvdata/bhyvexml2argv-msrs.xml  | 26 ++
> > > > >  tests/bhyvexml2argvtest.c |  1 +
> > > > >  .../bhyvexml2xmlout-msrs.xml  | 36 
> > > > > +++
> > > > >  tests/bhyvexml2xmltest.c  |  1 +
> > > > >  14 files changed, 165 insertions(+)
> > > > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args
> > > > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs
> > > > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.xml
> > > > >  create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-msrs.xml
> > > > > 
> > > > 
> > > > The code looks fine to me but this needs a bit more context.
> > > > Particularly 'ignore' is a bit ambiguous here so I had to dig
> > > > 
> > > > kvm+linux has arguably always ignored unknown MSRs but historically it
> > > > would print an error in host dmesg which often confused users quite a
> > > > bit. In the bhyve case it seems that unknown MSRs cause the VM to
> > > > essentially crash which is a pretty intense reaction. This option
> > > > disables that crashing behavior. So for this feature to be really
> > > > descriptive it would be  or something
> > > > like that
> > > 
> > > That is not actually the case for KVM.   When KVM sees an unhandled
> > > MSR it will inject a GPF to the guest, which will usually cause the
> > > guest to crash. KVM does this GPF injection for both reads and writes
> > > of unknown MSRs.
> > > 
> > > There is an opt-in kvm.ko  option "ignore-msrs" which tells KVM
> > > to silently ignore unknown MSRs instead. Ideally the KVM option
> > > would be exposed to QEMU as an option, since when people do need
> > > it, they usually only need it for a single guest.
> > > 
> > > > The bhyve man page says:
> > > > 
> > > > https://www.freebsd.org/cgi/man.cgi?query=bhyve&sektion=8
> > > > 
> > > > -wIgnore accesses to unimplemented Model Specific Registers
> > > >   (MSRs). This is intended for debug purposes.
> > > 
> > > I'm curious whether this applies to boths reads & writes of unknown
> > > MSRs, or just to writes.  '-w' naming suggests the latter, but the
> > > descrption "accesses" would suggest reads too.
> > 
> > From what I can see by briefly looking at the code, it applies to both
> > reads and writes:
> > 
> > https://svnweb.freebsd.org/base/head/usr.sbin/bhyve/bhyverun.c?revision=343642&view=markup#l520
> > 
> > Check two functions below this line.
> 
> Yes, I agree. This is good as its the same behaviour as KVM.
> 
> How about we change the XML to
> 
>   

Sounds good to me, I'll send v2 soon.

> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread Erik Skultety
On Thu, Feb 07, 2019 at 04:24:13PM +0100, Andrea Bolognani wrote:
> On Thu, 2019-02-07 at 07:12 -0500, John Ferlan wrote:
> > On 2/7/19 4:10 AM, Erik Skultety wrote:
> > > On Wed, Feb 06, 2019 at 08:41:33AM -0500, John Ferlan wrote:
> > > > +VIR_DEFINE_AUTOPTR_FUNC(virStorageAuthDef, virStorageAuthDefFree);
> > >
> > > ^This defines a static function, so the ';' is unnecessary, it doesn't 
> > > hurt,
> > > but you know, consistency ;). Also, keep the original newline below.
> >
> > Perhaps because I had recently reviewed Cole's series about removing the
>
> You meant *adding* here, right?
>
> > semicolon from VIR_ENUM_DECL, VIR_ENUM_IMPL, VIR_LOG_INIT, and
> > VIR_ONCE_GLOBAL_INIT - it was just "fresh" in my mind that macros should
> > have semicolons (moreso than actually looking at that definition).  I
> > can change them all.
>
> Please keep the semicolon! If a macro is used like a function, then
> its call sites should also look like those of a function.

Except VIR_DEFINE_AUTOPTR_FUNC macro is not used like a function, it defines a
function.

Erik

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


Re: [libvirt] [PATCH 10/10] qemu: caps: Don't call 'query-events' when we probe them from QMP schema

2019-02-07 Thread Eric Blake
On 2/7/19 6:28 AM, Peter Krempa wrote:
> Avoid calling the command and fix test fallout.
> 
> Note that for clarity and size this does not include the fix to the
> numbering of commands. I've used the following command to fix them and
> it makes tests pass:
> 
> for i in tests/qemucapabilitiesdata/*.replies; do ./tests/qemucapsfixreplies 
> $i; done
> ---
>  src/qemu/qemu_capabilities.c  |  10 +-
>  .../caps_2.10.0.aarch64.replies   | 122 
>  .../caps_2.10.0.ppc64.replies | 122 
>  .../caps_2.10.0.s390x.replies | 122 
>  .../caps_2.10.0.x86_64.replies| 122 
>  .../caps_2.11.0.s390x.replies | 122 
>  .../caps_2.11.0.x86_64.replies| 122 
>  .../caps_2.12.0.aarch64.replies   | 128 
>  .../caps_2.12.0.ppc64.replies | 128 
>  .../caps_2.12.0.s390x.replies | 128 
>  .../caps_2.12.0.x86_64.replies| 128 
>  .../caps_2.5.0.x86_64.replies | 116 ---
>  .../caps_2.6.0.aarch64.replies| 122 
>  .../caps_2.6.0.ppc64.replies  | 122 
>  .../caps_2.6.0.x86_64.replies | 122 
>  .../caps_2.7.0.s390x.replies  | 122 
>  .../caps_2.7.0.x86_64.replies | 122 
>  .../caps_2.8.0.s390x.replies  | 122 
>  .../caps_2.8.0.x86_64.replies | 122 
>  .../caps_2.9.0.ppc64.replies  | 122 
>  .../caps_2.9.0.s390x.replies  | 122 
>  .../caps_2.9.0.x86_64.replies | 122 
>  .../caps_3.0.0.ppc64.replies  | 131 -
>  .../caps_3.0.0.riscv32.replies| 134 -
>  .../caps_3.0.0.riscv64.replies| 134 -
>  .../caps_3.0.0.s390x.replies  | 134 -
>  .../caps_3.0.0.x86_64.replies | 134 -
>  .../caps_3.1.0.ppc64.replies  | 137 --
>  .../caps_3.1.0.x86_64.replies | 137 --
>  .../caps_4.0.0.riscv32.replies| 137 --
>  .../caps_4.0.0.riscv64.replies| 137 --
>  .../caps_4.0.0.x86_64.replies | 137 --
>  32 files changed, 4 insertions(+), 3938 deletions(-)

Impressive diffstat! Basically, for any pre-captured qemu output new
enough to reuse introspection results, we've gotten rid of the separate
query-events reply.

> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 9c79511b1d..93a06d3cd8 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2120,14 +2120,12 @@ virQEMUCapsProbeQMPEvents(virQEMUCapsPtr qemuCaps,
>  char **events = NULL;
>  int nevents;
> 
> -if ((nevents = qemuMonitorGetEvents(mon, &events)) < 0)
> -return -1;
> -
>  /* we can probe events also from the QMP schema so we can skip this here 
> */
> -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA)) {
> -virStringListFreeCount(events, nevents);
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA))
>  return 0;
> -}
> +
> +if ((nevents = qemuMonitorGetEvents(mon, &events)) < 0)
> +return -1;
> 
>  virQEMUCapsProcessStringFlags(qemuCaps,
>ARRAY_CARDINALITY(virQEMUCapsEvents),

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH 09/10] qemu: caps: Probe events from 'query-qmp-schema' rather than 'query-events'

2019-02-07 Thread Eric Blake
On 2/7/19 6:28 AM, Peter Krempa wrote:
> QEMU plans to deprecate 'query-events' as it's non-extensible. Events
> are also described by 'query-qmp-schema' so we can use that one instead.
> 
> This patch adds detection of events to
> virQEMUCapsProbeQMPSchemaCapabilities using the same structure declaring
> them for the old approach (virQEMUCapsEvents). This is possible as the
> name is the same in the QMP schema and our detector supports that
> trivially.
> 
> For any complex queries virQEMUCapsQMPSchemaQueries can be used in the
> future.
> 
> For now we still call 'query-events' and discard the result so that it's
> obvious that the tests pass. This will be cleaned up later.
> 
> Signed-off-by: Peter Krempa 
> ---
>  src/qemu/qemu_capabilities.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 7f3f87c48d..9c79511b1d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2123,6 +2123,12 @@ virQEMUCapsProbeQMPEvents(virQEMUCapsPtr qemuCaps,
>  if ((nevents = qemuMonitorGetEvents(mon, &events)) < 0)
>  return -1;
> 
> +/* we can probe events also from the QMP schema so we can skip this here 
> */
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA)) {
> +virStringListFreeCount(events, nevents);
> +return 0;
> +}

Slick. For this patch, we call query-events unconditionally (as before),
but when qemu is new enough to also have introspection, we throw away
the results without using them...

> +
>  virQEMUCapsProcessStringFlags(qemuCaps,
>ARRAY_CARDINALITY(virQEMUCapsEvents),
>virQEMUCapsEvents,
> @@ -4138,6 +4144,14 @@ virQEMUCapsProbeQMPSchemaCapabilities(virQEMUCapsPtr 
> qemuCaps,
>  virQEMUCapsSet(qemuCaps, entry->flag);
>  }
> 
> +/* probe also for basic event support */
> +for (i = 0; i < ARRAY_CARDINALITY(virQEMUCapsEvents); i++) {
> +entry = virQEMUCapsEvents + i;
> +
> +if (virQEMUQAPISchemaPathExists(entry->value, schema))
> +virQEMUCapsSet(qemuCaps, entry->flag);
> +}
> +

...and rebuild the same results from the introspection. I like the
split, and am glad that it was fairly trivial to update to introspection
while still keeping the fallback to query-events on older qemu.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



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

Re: [libvirt] [PATCH v2 23/32] qemu: caps: Always assume presence of 'ide-hd' and 'ide-cd' devices

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:56PM +0100, Peter Krempa wrote:

The split of scsi-disk into the two separate devices was introduced by


s/scsi-disk/ide-drive/


qemu commit 1f56e32a7f4b3 released in qemu v0.15.

Note that when compared to the previous commit which made sure that no
disk related tests were touched, in this case it's not as careful.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c  |  1 -
src/qemu/qemu_capabilities.h  |  2 +-
src/qemu/qemu_command.c   | 36 +++
.../caps_1.5.3.x86_64.xml |  1 -
.../caps_1.6.0.x86_64.xml |  1 -
.../caps_1.7.0.x86_64.xml |  1 -
.../caps_2.1.1.x86_64.xml |  1 -
.../caps_2.10.0.aarch64.xml   |  1 -
.../caps_2.10.0.ppc64.xml |  1 -
.../caps_2.10.0.x86_64.xml|  1 -
.../caps_2.11.0.x86_64.xml|  1 -
.../caps_2.12.0.aarch64.xml   |  1 -
.../caps_2.12.0.ppc64.xml |  1 -
.../caps_2.12.0.x86_64.xml|  1 -
.../caps_2.4.0.x86_64.xml |  1 -
.../caps_2.5.0.x86_64.xml |  1 -
.../caps_2.6.0.aarch64.xml|  1 -
.../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  1 -
.../caps_2.6.0.x86_64.xml |  1 -
.../caps_2.7.0.x86_64.xml |  1 -
.../caps_2.8.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  1 -
.../caps_2.9.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 -
.../caps_3.0.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 -
.../caps_3.1.0.x86_64.xml |  1 -
.../caps_4.0.0.riscv32.xml|  1 -
.../caps_4.0.0.riscv64.xml|  1 -
.../caps_4.0.0.x86_64.xml |  1 -
tests/qemuxml2argvdata/autoindex.args |  2 +-
.../qemuxml2argvdata/balloon-device-auto.args |  3 +-

[...]

tests/qemuxml2argvdata/watchdog-device.args   |  3 +-
tests/qemuxml2argvdata/watchdog-dump.args |  3 +-
.../qemuxml2argvdata/watchdog-injectnmi.args  |  3 +-
tests/qemuxml2argvdata/watchdog.args  |  3 +-
tests/qemuxml2argvtest.c  | 10 ++
293 files changed, 358 insertions(+), 654 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 22/32] qemu: caps: Always assume presence of 'scsi-hd' and 'scsi-cd' device

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:55PM +0100, Peter Krempa wrote:

The split of scsi-disk into the two separate devices was introduced by
qemu commit b443ae67 released in qemu v0.15.

All changes to test files are not really related to disk testing thanks
to previous refactors.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c  |  1 -
src/qemu/qemu_capabilities.h  |  2 +-
src/qemu/qemu_command.c   | 19 ++-
.../caps_1.5.3.x86_64.xml |  1 -
.../caps_1.6.0.x86_64.xml |  1 -
.../caps_1.7.0.x86_64.xml |  1 -
.../caps_2.1.1.x86_64.xml |  1 -
.../caps_2.10.0.aarch64.xml   |  1 -
.../caps_2.10.0.ppc64.xml |  1 -
.../caps_2.10.0.s390x.xml |  1 -
.../caps_2.10.0.x86_64.xml|  1 -
.../caps_2.11.0.s390x.xml |  1 -
.../caps_2.11.0.x86_64.xml|  1 -
.../caps_2.12.0.aarch64.xml   |  1 -
.../caps_2.12.0.ppc64.xml |  1 -
.../caps_2.12.0.s390x.xml |  1 -
.../caps_2.12.0.x86_64.xml|  1 -
.../caps_2.4.0.x86_64.xml |  1 -
.../caps_2.5.0.x86_64.xml |  1 -
.../caps_2.6.0.aarch64.xml|  1 -
.../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  1 -
.../caps_2.6.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_2.7.0.s390x.xml |  1 -
.../caps_2.7.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_2.8.0.s390x.xml |  1 -
.../caps_2.8.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  1 -
.../qemucapabilitiesdata/caps_2.9.0.s390x.xml |  1 -
.../caps_2.9.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 -
.../caps_3.0.0.riscv32.xml|  1 -
.../caps_3.0.0.riscv64.xml|  1 -
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml |  1 -
.../caps_3.0.0.x86_64.xml |  1 -
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  1 -
.../caps_3.1.0.x86_64.xml |  1 -
.../caps_4.0.0.riscv32.xml|  1 -
.../caps_4.0.0.riscv64.xml|  1 -
.../caps_4.0.0.x86_64.xml |  1 -
.../aarch64-virtio-pci-manual-addresses.args  |  4 ++--
tests/qemuxml2argvdata/bios-nvram-secure.args |  4 ++--
tests/qemuxml2argvdata/machine-smm-opt.args   |  4 ++--
.../multifunction-pci-device.args |  2 +-
.../pseries-vio-user-assigned.args|  4 ++--
tests/qemuxml2argvdata/pseries-vio.args   |  4 ++--
tests/qemuxml2argvtest.c  | 11 ---
tests/qemuxml2xmltest.c   |  6 ++
47 files changed, 24 insertions(+), 73 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 21/32] tests: qemuxml2argv: Remove 'disk-virtio-scsi-ccw' test

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:54PM +0100, Peter Krempa wrote:

It's a subset of 'iothreads-virtio-scsi-ccw'.

Signed-off-by: Peter Krempa 
---
.../disk-virtio-scsi-ccw.args | 29 -
.../qemuxml2argvdata/disk-virtio-scsi-ccw.xml | 31 ---
tests/qemuxml2argvtest.c  |  2 --
3 files changed, 62 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-ccw.args
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-ccw.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 20/32] tests: qemuxml2argv: Modernize virtio-scsi iothread tests

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:53PM +0100, Peter Krempa wrote:

Use DO_TEST_CAPS_LATEST to obtain actual results.



s/actual/modern/


Signed-off-by: Peter Krempa 
---
...threads-virtio-scsi-ccw.s390x-latest.args} | 20 ++--
...hreads-virtio-scsi-pci.x86_64-latest.args} | 23 +++
tests/qemuxml2argvtest.c  |  8 ++-
3 files changed, 29 insertions(+), 22 deletions(-)
rename tests/qemuxml2argvdata/{iothreads-virtio-scsi-ccw.args => 
iothreads-virtio-scsi-ccw.s390x-latest.args} (58%)
rename tests/qemuxml2argvdata/{iothreads-virtio-scsi-pci.args => 
iothreads-virtio-scsi-pci.x86_64-latest.args} (57%)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread Andrea Bolognani
On Thu, 2019-02-07 at 07:12 -0500, John Ferlan wrote:
> On 2/7/19 4:10 AM, Erik Skultety wrote:
> > On Wed, Feb 06, 2019 at 08:41:33AM -0500, John Ferlan wrote:
> > > +VIR_DEFINE_AUTOPTR_FUNC(virStorageAuthDef, virStorageAuthDefFree);
> > 
> > ^This defines a static function, so the ';' is unnecessary, it doesn't hurt,
> > but you know, consistency ;). Also, keep the original newline below.
> 
> Perhaps because I had recently reviewed Cole's series about removing the

You meant *adding* here, right?

> semicolon from VIR_ENUM_DECL, VIR_ENUM_IMPL, VIR_LOG_INIT, and
> VIR_ONCE_GLOBAL_INIT - it was just "fresh" in my mind that macros should
> have semicolons (moreso than actually looking at that definition).  I
> can change them all.

Please keep the semicolon! If a macro is used like a function, then
its call sites should also look like those of a function.

-- 
Andrea Bolognani / Red Hat / Virtualization

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


Re: [libvirt] [PATCH v2 19/32] tests: qemuxml2argv: Use 1.5.3 version for very old state in 'disk-cache'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:52PM +0100, Peter Krempa wrote:

Rather than testing random set of flags add a case also for the oldest
supported qemu.



s/very old state in/the oldest case of/
or something sensible


Signed-off-by: Peter Krempa 
---
...k-cache.args => disk-cache.x86_64-1.5.3.args} | 16 +---
tests/qemuxml2argvtest.c |  2 +-
2 files changed, 10 insertions(+), 8 deletions(-)
rename tests/qemuxml2argvdata/{disk-cache.args => disk-cache.x86_64-1.5.3.args} 
(72%)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [jenkins-ci PATCH v2 8/9] lcitool: support generating cross compiler dockerfiles

2019-02-07 Thread Andrea Bolognani
On Tue, 2019-02-05 at 17:53 +, Daniel P. Berrangé wrote:
[...]
> With the Debian 9 distro, this supports arm64, armel, armhf, mips,
> mipsel, mips64el, ppc64el, s390x, which are all the official archs
> that Debian maintainers currently build libvirt for. With Debian Sid,
> this is extended to include i386.

Is i386 really not supported as a cross compilation target on Debian
9? It seems like it would be such a low hanging fruit...

[...]
> +++ b/guests/host_vars/libvirt-debian-9/docker.yml
> @@ -1,2 +1,59 @@
>  ---
>  docker_base: debian:9
> +cross_build:

I don't think this belongs in docker.yml, since there's nothing
specific to Docker here: we could just as well use this information
to build a virtual machine in which to perform cross builds.

I would create a new fact file, eg. cross-build.yml, for this.

To be completely honest I'm not 100% sold on the structure and
placement of the data, but I can't quite put my fingers on why
that's the case - which of course makes it very difficult to come
up with an alternative suggestion ;) So let's use this structure
for now, we can always change it later if needed.

> +  blacklist:
> +# Doesn't properly resolve from foreign arch package
> +# to the native package of augeas-lenses

I cannot really understand what you're trying to say with this
comment, can you try to reword it please?

> +- libnetcf-dev
> +# Fails to resolve deps from foreign arch
> +- wireshark-dev
> +# dtrace tool doesn't know how to cross-compile
> +- systemtap-sdt-dev

For these and all other blacklisted packages, you should list the
name of the mapping (eg. netcf) rather than the concrete Debian
package name (eg. libnetcf-dev). Then of course you'll have to act
on the blacklist earlier, before mapping is performed.

> +  arches:
> +arm64:
> +  gcc-pkg-prefix: crossbuild-essential-arm64

This is not a prefix, and the package is not GCC :)

I would use 'cross_gcc', which incidentally is also the name of the
variable you use in the Python script to store this value. (More on
this later, though.)

> +  target-prefix: aarch64-linux-gnu

Same here, you later use 'cross_prefix' in the script but also store
it as 'TARGET' in the environment... I'd suggest 'cross_target'.

[...]
> +mipsel:
> +  gcc-pkg-prefix: gcc-mipsel-linux-gnu

crossbuild-essential-mipsel is available in Debian 9, so you should
use that instead.

[...]
> +++ b/guests/host_vars/libvirt-debian-sid/docker.yml
> @@ -1,2 +1,64 @@
>  ---
>  docker_base: debian:sid
> +cross_build:
> +  blacklist:
> +# Doesn't properly resolve from foreign arch package
> +# to the native package of augeas-lenses
> +- libnetcf-dev
> +# Fails to resolve deps from foreign arch
> +- wireshark-dev
> +# dtrace tool doesn't know how to cross-compile
> +- systemtap-sdt-dev
> +# appears to be dropped from the 'sid' tree
> +- sheepdog

So it has! And it's apparently not going to be in the next Ubuntu
release, either.

This has nothing to do with cross compilation, though: you should
just update the regular mappings file, in a separate commit, based
on this information.

[...]
> +mips64el:
> +  gcc-pkg-prefix: gcc-mips64el-linux-gnuabi64

Debian Sid has crossbuild-essential-* packages available for all
cross compilation targets, so please use those everywhere.

Another idea would be to drop these from the facts entirely and
have a bunch of mappings like

  cross-gcc-mipsel:
Debian9: gcc-mipsel-linux-gnu
DebianSid: crossbuild-essential-mipsel

instead; then you could programmatically add cross-gcc-$arch to
the package list in the script before mapping is performed. That
sounds to me like it would be much nicer.

[...]
> +++ b/guests/lcitool
> @@ -343,6 +343,11 @@ class Application:
>  metavar="GIT_REVISION",
>  help="git revision to build (remote/branch)",
>  )
> +self._parser.add_argument(
> +"-x",
> +metavar="CROSS-ARCH",
> +help="cross compiler architecture (dockerfile action only)",
> +)

s/CROSS-ARCH/CROSS_ARCH/

[...]
> @@ -501,10 +506,24 @@ class Application:
>  os_name = facts["os_name"]
>  os_version = facts["os_version"]
>  os_full = os_name + os_version
> +blacklist = []
> +cross_build_facts = None
>  
>  if package_format not in ["deb", "rpm"]:
>  raise Error("Host {} doesn't support Dockerfiles".format(host))
>  
> +if cross_arch is not None:
> +if package_format != "deb":
> +raise Error("cross compilers only supported for debian 
> packages")

This first check is kinda pointless, since the one right after it
would be triggered when trying to generate a Dockerfile for cross
compilation and the required information has not been provided by
maintainers.

> +if "cross_build" not in facts:
> +raise Error("cross compilers not supported for this host"

[libvirt] [PATCH] virinitctl: Provide a stub list of init fifos for non-Linux

2019-02-07 Thread Michal Privoznik
The virInitctlFifos list is exported, but lacks definition for
non-Linux and/or non-BSD case.

Signed-off-by: Michal Privoznik 
---

Pushed under build breaker rule.

 src/util/virinitctl.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index bbcbbb483d..6966e8a3e9 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -184,6 +184,11 @@ virInitctlSetRunLevel(const char *fifo,
 return ret;
 }
 #else
+/* On non-Linux and non-BSD there are no known inits */
+const char *virInitctlFifos[] = {
+  NULL
+};
+
 int virInitctlSetRunLevel(const char *fifo ATTRIBUTE_UNUSED,
   virInitctlRunLevel level ATTRIBUTE_UNUSED)
 {
-- 
2.19.2

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


Re: [libvirt] [PATCH v2 7/8] qemu: Implement VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 04:11:46PM +0100, Jiri Denemark wrote:

This flag tells virDomainMigrateSetMaxSpeed and
virDomainMigrateGetMaxSpeed APIs to work on post-copy migration
bandwidth.

Signed-off-by: Jiri Denemark 
---

Notes:
   Version 2:
   - if (postcopy) branch from qemuDomainMigrateGetMaxSpeed separated
 into a helper function

src/qemu/qemu_driver.c | 110 -
1 file changed, 97 insertions(+), 13 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 18/32] tests: qemu: Merge 'disk-scsi-mptsas1068' test into 'disk-scsi'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:51PM +0100, Peter Krempa wrote:

As we support multiple scsi controllers there's no need to have a
special test for this controller.

Signed-off-by: Peter Krempa 


In the commit summary:
s/mptsas1068/vscsi/g


---
tests/qemuxml2argvdata/disk-scsi-vscsi.args   | 31 -
tests/qemuxml2argvdata/disk-scsi-vscsi.xml| 35 ---
.../disk-scsi.x86_64-latest.args  |  4 ++
tests/qemuxml2argvdata/disk-scsi.xml  |  6 +++
tests/qemuxml2argvtest.c  |  1 -
tests/qemuxml2xmloutdata/disk-scsi-vscsi.xml  | 45 ---
tests/qemuxml2xmloutdata/disk-scsi.xml|  9 
tests/qemuxml2xmltest.c   |  1 -
8 files changed, 19 insertions(+), 113 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-vscsi.args
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-vscsi.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-scsi-vscsi.xml



Reviewed-by: Ján Tomko 

Jano


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

[libvirt] [PATCH v2 7/8] qemu: Implement VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag

2019-02-07 Thread Jiri Denemark
This flag tells virDomainMigrateSetMaxSpeed and
virDomainMigrateGetMaxSpeed APIs to work on post-copy migration
bandwidth.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 2:
- if (postcopy) branch from qemuDomainMigrateGetMaxSpeed separated
  into a helper function

 src/qemu/qemu_driver.c | 110 -
 1 file changed, 97 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0c0ed4fab9..7106f6d553 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14275,10 +14275,12 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom,
 virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
 qemuDomainObjPrivatePtr priv;
-int rc;
+bool postcopy = !!(flags & VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY);
+VIR_AUTOPTR(qemuMigrationParams) migParams = NULL;
+unsigned long long max;
 int ret = -1;
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, -1);
 
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
@@ -14288,14 +14290,18 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom,
 if (virDomainMigrateSetMaxSpeedEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (bandwidth > QEMU_DOMAIN_MIG_BANDWIDTH_MAX) {
+if (postcopy)
+max = ULLONG_MAX / 1024 / 1024;
+else
+max = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
+
+if (bandwidth > max) {
 virReportError(VIR_ERR_OVERFLOW,
-   _("bandwidth must be less than %llu"),
-   QEMU_DOMAIN_MIG_BANDWIDTH_MAX + 1ULL);
+   _("bandwidth must be less than %llu"), max + 1);
 goto cleanup;
 }
 
-if (!virDomainObjIsActive(vm)) {
+if (!postcopy && !virDomainObjIsActive(vm)) {
 priv->migMaxBandwidth = bandwidth;
 ret = 0;
 goto cleanup;
@@ -14308,12 +14314,29 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom,
 goto endjob;
 
 VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth);
-qemuDomainObjEnterMonitor(driver, vm);
-rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth);
-if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
-goto endjob;
 
-priv->migMaxBandwidth = bandwidth;
+if (postcopy) {
+if (!(migParams = qemuMigrationParamsNew()))
+goto endjob;
+
+if (qemuMigrationParamsSetULL(migParams,
+  
QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH,
+  bandwidth * 1024 * 1024) < 0)
+goto endjob;
+
+if (qemuMigrationParamsApply(driver, vm, QEMU_ASYNC_JOB_NONE,
+ migParams) < 0)
+goto endjob;
+} else {
+int rc;
+
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth);
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+goto endjob;
+
+priv->migMaxBandwidth = bandwidth;
+}
 
 ret = 0;
 
@@ -14325,16 +14348,71 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom,
 return ret;
 }
 
+
+static int
+qemuDomainMigrationGetPostcopyBandwidth(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+unsigned long *bandwidth)
+{
+VIR_AUTOPTR(qemuMigrationParams) migParams = NULL;
+unsigned long long bw;
+int rc;
+int ret = -1;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+return -1;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto cleanup;
+
+if (qemuMigrationParamsFetch(driver, vm, QEMU_ASYNC_JOB_NONE,
+ &migParams) < 0)
+goto cleanup;
+
+if ((rc = qemuMigrationParamsGetULL(migParams,
+
QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH,
+&bw)) < 0)
+goto cleanup;
+
+if (rc == 1) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("querying maximum post-copy migration speed is "
+ "not supported by QEMU binary"));
+goto cleanup;
+}
+
+/* QEMU reports B/s while we use MiB/s */
+bw /= 1024 * 1024;
+
+if (bw > ULONG_MAX) {
+virReportError(VIR_ERR_OVERFLOW,
+   _("bandwidth %llu is greater than %lu which is the "
+ "maximum value supported by this API"),
+   bw, ULONG_MAX);
+goto cleanup;
+}
+
+*bandwidth = bw;
+ret = 0;
+
+ cleanup:
+qemuDomainObjEndJob(driver, vm);
+return ret;
+}
+
+
 static int
 qemuDomainMigrateGetMaxSpeed(virDomainPtr dom,
  unsigned long *bandwidth,
  unsigned int flags)
 {
+virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr 

Re: [libvirt] [PATCH v2 17/32] tests: qemu: Merge 'disk-scsi-mptsas1068' test into 'disk-scsi'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:50PM +0100, Peter Krempa wrote:

As we support multiple scsi controllers there's no need to have a
special test for this controller.

Signed-off-by: Peter Krempa 
---
.../disk-scsi-mptsas1068.args | 31 -
.../qemuxml2argvdata/disk-scsi-mptsas1068.xml | 36 ---
.../disk-scsi.x86_64-latest.args  |  6 ++-
tests/qemuxml2argvdata/disk-scsi.xml  |  7 +++
tests/qemuxml2argvtest.c  |  3 --
.../disk-scsi-mptsas1068.xml  | 46 ---
tests/qemuxml2xmloutdata/disk-scsi.xml| 12 -
tests/qemuxml2xmltest.c   |  6 +--
8 files changed, 25 insertions(+), 122 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-mptsas1068.args
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-mptsas1068.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-scsi-mptsas1068.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 16/32] tests: qemu: Merge 'disk-scsi-megasas' test into 'disk-scsi'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:49PM +0100, Peter Krempa wrote:

As we support multiple scsi controllers there's no need to have a
special test for this controller.

Signed-off-by: Peter Krempa 
---
tests/qemuxml2argvdata/disk-scsi-megasas.args | 31 -
tests/qemuxml2argvdata/disk-scsi-megasas.xml  | 35 ---
.../disk-scsi.x86_64-latest.args  |  6 ++-
tests/qemuxml2argvdata/disk-scsi.xml  |  6 +++
tests/qemuxml2argvtest.c  |  2 -
.../qemuxml2xmloutdata/disk-scsi-megasas.xml  | 45 ---
tests/qemuxml2xmloutdata/disk-scsi.xml| 11 -
tests/qemuxml2xmltest.c   |  4 +-
8 files changed, 22 insertions(+), 118 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-megasas.args
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-megasas.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-scsi-megasas.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 12/15] test: Use VIR_AUTOFREE for test driver

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:44AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
...

>
> @@ -6342,7 +6302,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
>  virDomainSnapshotObjPtr snap = NULL;
>  virDomainSnapshotPtr snapshot = NULL;
>  virObjectEventPtr event = NULL;
> -char *xml = NULL;
> +VIR_AUTOFREE(char *) xml = NULL;

@xml will become unused in this function with this conversion, so I'd say cook
up a trivial patch that will get rid of it as unused.

Reviewed-by: Erik Skultety 

>  bool update_current = true;
>  bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
>  unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
> @@ -6427,7 +6387,6 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
>
>  snapshot = virGetDomainSnapshot(domain, snap->def->name);
>   cleanup:
> -VIR_FREE(xml);
>  if (vm) {
>  if (snapshot) {
>  virDomainSnapshotObjPtr other;

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


Re: [libvirt] [PATCH 14/15] storage: Use VIR_AUTOCLOSE

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:46AM -0500, John Ferlan wrote:
> Modify code to use the VIR_AUTOCLOSE logic cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH v2 15/32] tests: qemuxml2argv: Modernize 'disk-scsi' test

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:48PM +0100, Peter Krempa wrote:

Use DO_TEST_CAPS_LATEST rather than a predetermined set of caps.

Signed-off-by: Peter Krempa 
---
tests/qemuxml2argvdata/disk-scsi.args | 30 
.../disk-scsi.x86_64-latest.args  | 35 +++
tests/qemuxml2argvtest.c  |  3 +-
3 files changed, 36 insertions(+), 32 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-scsi.args
create mode 100644 tests/qemuxml2argvdata/disk-scsi.x86_64-latest.args



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 14/32] tests: qemu: Rename 'disk-scsi-device' to 'disk-scsi'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:47PM +0100, Peter Krempa wrote:

Drop the 'device' suffix which is quite pointless.

Signed-off-by: Peter Krempa 
---
.../qemuxml2argvdata/{disk-scsi-device.args => disk-scsi.args} | 0
tests/qemuxml2argvdata/{disk-scsi-device.xml => disk-scsi.xml} | 0
tests/qemuxml2argvtest.c   | 2 +-
.../qemuxml2xmloutdata/{disk-scsi-device.xml => disk-scsi.xml} | 0
tests/qemuxml2xmltest.c| 3 +--
5 files changed, 2 insertions(+), 3 deletions(-)
rename tests/qemuxml2argvdata/{disk-scsi-device.args => disk-scsi.args} (100%)
rename tests/qemuxml2argvdata/{disk-scsi-device.xml => disk-scsi.xml} (100%)
rename tests/qemuxml2xmloutdata/{disk-scsi-device.xml => disk-scsi.xml} (100%)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 13/32] tests: qemuxml2argv: Move cases from 'disk-shared-locking' into 'disk-shared'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:46PM +0100, Peter Krempa wrote:

The tests are for the same feature. Move all the cases to 'disk-shared'
case as it's already using DO_TEST_CAPS_LATEST

Signed-off-by: Peter Krempa 
---
.../qemuxml2argvdata/disk-shared-locking.args | 34 ---
.../qemuxml2argvdata/disk-shared-locking.xml  | 42 ---
.../disk-shared.x86_64-2.12.0.args|  9 +++-
.../disk-shared.x86_64-latest.args|  9 +++-
tests/qemuxml2argvdata/disk-shared.xml| 14 +++
tests/qemuxml2argvtest.c  |  2 -
6 files changed, 30 insertions(+), 80 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-shared-locking.args
delete mode 100644 tests/qemuxml2argvdata/disk-shared-locking.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 12/32] tests: qemuxml2argv: Remove the 'after startup XML' testing machinery

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:45PM +0100, Peter Krempa wrote:

A lot of code with no real impact and popularity. Remove all the helpers
now that the only test case is gone.



Bye, machinery!


Signed-off-by: Peter Krempa 
---
tests/Makefile.am|  1 -
tests/qemuxml2argvtest.c | 54 +++-
2 files changed, 3 insertions(+), 52 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 13/15] tests: Use VIR_AUTOFREE for various storage tests

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:45AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH v2 11/32] tests: qemuxml2argv: Remove testing of post startup change to 'cachemode' for shared disks

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:44PM +0100, Peter Krempa wrote:

Testing that the cachemode is properly recorded to the configuration
after startup does not add much value and overcomplicates the xml2argv
test.

Remove the 'disk-shared' test with old capabilities, which would be done
along as well as the test with real capabilities covers it sufficiently.



-EPARSE

s/along //


Signed-off-by: Peter Krempa 
---
tests/qemuxml2argvdata/disk-shared.args   | 31 --
tests/qemuxml2argvtest.c  |  1 -
.../qemuxml2startupxmloutdata/disk-shared.xml | 56 ---
3 files changed, 88 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-shared.args
delete mode 100644 tests/qemuxml2startupxmloutdata/disk-shared.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 10/32] tests: qemuxml2argv: Use real caps when auto-generating SCSI controller type

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:43PM +0100, Peter Krempa wrote:

Using an old strict set of capabilities is not of much use if a code
path would select a more modern controller on accident.


by accident?



Signed-off-by: Peter Krempa 
---
...> disk-scsi-device-auto.x86_64-1.5.3.args} | 11 +++---
.../disk-scsi-device-auto.x86_64-latest.args  | 35 +++
tests/qemuxml2argvtest.c  |  4 +--
3 files changed, 43 insertions(+), 7 deletions(-)
rename tests/qemuxml2argvdata/{disk-scsi-device-auto.args => 
disk-scsi-device-auto.x86_64-1.5.3.args} (69%)
create mode 100644 
tests/qemuxml2argvdata/disk-scsi-device-auto.x86_64-latest.args



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 09/32] tests: qemuxml: Merge 'ioeventfd' variant of 'virtio-scsi' test into the common file

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:42PM +0100, Peter Krempa wrote:

We don't need separate files for this test. Also modernize it in the
process.

Signed-off-by: Peter Krempa 
---
.../controller-virtio-scsi.x86_64-latest.args |  6 ++-
.../controller-virtio-scsi.xml|  8 
.../disk-virtio-scsi-ioeventfd.args   | 28 --
.../disk-virtio-scsi-ioeventfd.xml| 31 
tests/qemuxml2argvtest.c  |  2 -
.../controller-virtio-scsi.xml| 12 +-
.../disk-virtio-scsi-ioeventfd.xml| 37 ---
tests/qemuxml2xmltest.c   |  2 -
8 files changed, 24 insertions(+), 102 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-ioeventfd.args
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-ioeventfd.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-virtio-scsi-ioeventfd.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 08/32] tests: qemuxml: Merge 'max_sectors' variant of 'virtio-scsi' test into the common file

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:41PM +0100, Peter Krempa wrote:

We don't need separate files for this test. Also modernize it in the
process.

Signed-off-by: Peter Krempa 
---
.../controller-virtio-scsi.x86_64-latest.args |  6 ++-
.../controller-virtio-scsi.xml|  8 
.../disk-virtio-scsi-max_sectors.args | 28 --
.../disk-virtio-scsi-max_sectors.xml  | 31 
tests/qemuxml2argvtest.c  |  2 -
.../controller-virtio-scsi.xml| 12 +-
.../disk-virtio-scsi-max_sectors.xml  | 37 ---
tests/qemuxml2xmltest.c   |  2 -
8 files changed, 24 insertions(+), 102 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-max_sectors.args
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-max_sectors.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-virtio-scsi-max_sectors.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 11/15] util: Use VIR_AUTOFREE for virstoragefile

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:43AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH v2 06/32] tests: qemuxml: Merge 'num-queues' variant of 'virtio-scsi' test into the common file

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:39PM +0100, Peter Krempa wrote:

We don't need separate files for this test. Also modernize it in the
process.

Signed-off-by: Peter Krempa 
---
.../controller-virtio-scsi.x86_64-latest.args |  6 ++-
.../controller-virtio-scsi.xml|  8 
.../disk-virtio-scsi-num_queues.args  | 28 --
.../disk-virtio-scsi-num_queues.xml   | 31 
tests/qemuxml2argvtest.c  |  2 -
.../controller-virtio-scsi.xml| 12 +-
.../disk-virtio-scsi-num_queues.xml   | 37 ---
tests/qemuxml2xmltest.c   |  2 -
8 files changed, 24 insertions(+), 102 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-num_queues.args
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-num_queues.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-virtio-scsi-num_queues.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 07/32] tests: qemuxml: Merge 'cmd_per_lun' variant of 'virtio-scsi' test into the common file

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:40PM +0100, Peter Krempa wrote:

We don't need separate files for this test. Also modernize it in the
process.

Signed-off-by: Peter Krempa 
---
.../controller-virtio-scsi.x86_64-latest.args |  6 ++-
.../controller-virtio-scsi.xml|  8 
.../disk-virtio-scsi-cmd_per_lun.args | 28 --
.../disk-virtio-scsi-cmd_per_lun.xml  | 31 
tests/qemuxml2argvtest.c  |  2 -
.../controller-virtio-scsi.xml| 12 +-
.../disk-virtio-scsi-cmd_per_lun.xml  | 37 ---
tests/qemuxml2xmltest.c   |  2 -
8 files changed, 24 insertions(+), 102 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-cmd_per_lun.args
delete mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-cmd_per_lun.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-virtio-scsi-cmd_per_lun.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 05/32] tests: qemu: Remove 'disk-scsi-virtio-scsi' test

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:38PM +0100, Peter Krempa wrote:

Now that we have a specific test for testing the 'virtio-scsi'
controller and other tests which test a combination of scsi and non-scsi
devices this test no longer makes sense.



Bye, test!


Signed-off-by: Peter Krempa 
---
.../disk-scsi-virtio-scsi.args| 31 -
.../disk-scsi-virtio-scsi.xml | 35 ---
tests/qemuxml2argvtest.c  |  2 -
.../disk-scsi-virtio-scsi.xml | 45 ---
tests/qemuxml2xmltest.c   |  2 -
5 files changed, 115 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-virtio-scsi.args
delete mode 100644 tests/qemuxml2argvdata/disk-scsi-virtio-scsi.xml
delete mode 100644 tests/qemuxml2xmloutdata/disk-scsi-virtio-scsi.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 0/3] bhyve: implement MSRs ignore unknown writes feature

2019-02-07 Thread Daniel P . Berrangé
On Thu, Feb 07, 2019 at 06:05:25PM +0400, Roman Bogorodskiy wrote:
>   Daniel P. Berrangé wrote:
> 
> > On Wed, Feb 06, 2019 at 04:00:06PM -0500, Cole Robinson wrote:
> > > On 1/25/19 12:54 PM, Roman Bogorodskiy wrote:
> > > > 
> > > > Roman Bogorodskiy (3):
> > > >   conf: introduce 'msrs' feature
> > > >   bhyve: implement MSRs ignore unknown writes feature
> > > >   news: document bhyve msrs feature
> > > > 
> > > >  docs/drvbhyve.html.in | 16 +
> > > >  docs/formatdomain.html.in |  1 +
> > > >  docs/news.xml | 11 ++
> > > >  docs/schemas/domaincommon.rng | 14 
> > > >  src/bhyve/bhyve_command.c |  4 +++
> > > >  src/conf/domain_conf.c| 33 +
> > > >  src/conf/domain_conf.h|  8 +
> > > >  src/qemu/qemu_domain.c|  1 +
> > > >  .../bhyvexml2argvdata/bhyvexml2argv-msrs.args | 10 ++
> > > >  .../bhyvexml2argv-msrs.ldargs |  3 ++
> > > >  .../bhyvexml2argvdata/bhyvexml2argv-msrs.xml  | 26 ++
> > > >  tests/bhyvexml2argvtest.c |  1 +
> > > >  .../bhyvexml2xmlout-msrs.xml  | 36 +++
> > > >  tests/bhyvexml2xmltest.c  |  1 +
> > > >  14 files changed, 165 insertions(+)
> > > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args
> > > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs
> > > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.xml
> > > >  create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-msrs.xml
> > > > 
> > > 
> > > The code looks fine to me but this needs a bit more context.
> > > Particularly 'ignore' is a bit ambiguous here so I had to dig
> > > 
> > > kvm+linux has arguably always ignored unknown MSRs but historically it
> > > would print an error in host dmesg which often confused users quite a
> > > bit. In the bhyve case it seems that unknown MSRs cause the VM to
> > > essentially crash which is a pretty intense reaction. This option
> > > disables that crashing behavior. So for this feature to be really
> > > descriptive it would be  or something
> > > like that
> > 
> > That is not actually the case for KVM.   When KVM sees an unhandled
> > MSR it will inject a GPF to the guest, which will usually cause the
> > guest to crash. KVM does this GPF injection for both reads and writes
> > of unknown MSRs.
> > 
> > There is an opt-in kvm.ko  option "ignore-msrs" which tells KVM
> > to silently ignore unknown MSRs instead. Ideally the KVM option
> > would be exposed to QEMU as an option, since when people do need
> > it, they usually only need it for a single guest.
> > 
> > > The bhyve man page says:
> > > 
> > > https://www.freebsd.org/cgi/man.cgi?query=bhyve&sektion=8
> > > 
> > > -wIgnore accesses to unimplemented Model Specific Registers
> > >   (MSRs). This is intended for debug purposes.
> > 
> > I'm curious whether this applies to boths reads & writes of unknown
> > MSRs, or just to writes.  '-w' naming suggests the latter, but the
> > descrption "accesses" would suggest reads too.
> 
> From what I can see by briefly looking at the code, it applies to both
> reads and writes:
> 
> https://svnweb.freebsd.org/base/head/usr.sbin/bhyve/bhyverun.c?revision=343642&view=markup#l520
> 
> Check two functions below this line.

Yes, I agree. This is good as its the same behaviour as KVM.

How about we change the XML to

  

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 10/15] conf: Use VIR_AUTOFREE for storage_conf

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:42AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH v2 04/32] tests: qemuxml: Add a common test file for the 'virtio-scsi' controller

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:37PM +0100, Peter Krempa wrote:

Add a file to agregate testing for 'virtio-scsi' based on the modern


aggregate


framework.

Signed-off-by: Peter Krempa 
---
.../controller-virtio-scsi.x86_64-latest.args | 34 ++
.../controller-virtio-scsi.xml| 29 +++
tests/qemuxml2argvtest.c  |  1 +
.../controller-virtio-scsi.xml| 36 +++
tests/qemuxml2xmltest.c   |  1 +
5 files changed, 101 insertions(+)
create mode 100644 
tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/controller-virtio-scsi.xml
create mode 100644 tests/qemuxml2xmloutdata/controller-virtio-scsi.xml



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 03/32] qemu: capabilities: Probe caps for 'ide-hd' instead of 'ide-drive'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:36PM +0100, Peter Krempa wrote:

Since commit a4cda054e7 we are using 'ide-hd' and 'ide-cd' instead of
'ide-drive'. We also should probe capabilities for 'ide-hd' instead of
'ide-drive'. It is safe to do as 'ide-drive' is the common denominator
of both 'ide-hd' and 'ide-cd' so all the properties were common.

For now the test data are modified by just changing the appropriate type
when probing for caps.

^ stray space



Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c   | 2 +-
tests/qemucapabilitiesdata/caps_1.5.3.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_1.6.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_1.7.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.1.1.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.replies | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.s390x.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.s390x.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.6.0.aarch64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.6.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.7.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.9.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.riscv32.replies  | 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.riscv64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_3.1.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies  | 2 +-
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.replies   | 2 +-
37 files changed, 37 insertions(+), 37 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 02/32] qemu: capabilities: Probe caps for 'scsi-hd' instead of 'scsi-disk'

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:35PM +0100, Peter Krempa wrote:

Since commit 02e8d0cfdf8 we are using 'scsi-hd' and 'scsi-cd' instead of
'scsi-disk'. We also should probe capabilities for 'scsi-hd' instead of
'scsi-disk'. It is safe to do as 'scsi-disk' is the common denominator
of both 'scsi-hd' and 'scsi-cd' so all the properties were common.

For now the test data are modified by just changing the appropriate type
when probing for caps.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_capabilities.c   | 2 +-
tests/qemucapabilitiesdata/caps_1.5.3.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_1.6.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_1.7.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.1.1.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.replies | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.s390x.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.s390x.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.6.0.aarch64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_2.6.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.7.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.9.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.riscv32.replies  | 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.riscv64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.s390x.replies| 2 +-
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_3.1.0.ppc64.replies| 2 +-
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.replies   | 2 +-
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.replies  | 2 +-
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.replies  | 2 +-
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.replies   | 2 +-
37 files changed, 37 insertions(+), 37 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 01/32] tests: qemucaps: Make fake 'microcodeVersion' depend on filename instead of length

2019-02-07 Thread Ján Tomko

On Mon, Feb 04, 2019 at 04:46:34PM +0100, Peter Krempa wrote:

To avoid changes to the filled in microcode in case we change the caps
replies file for any reason make the number depend on the filename.

Signed-off-by: Peter Krempa 
---
.../qemucapabilitiesdata/caps_1.5.3.x86_64.xml  |  2 +-
.../qemucapabilitiesdata/caps_1.6.0.x86_64.xml  |  2 +-
.../qemucapabilitiesdata/caps_1.7.0.x86_64.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.1.1.x86_64.xml  |  2 +-
.../caps_2.10.0.aarch64.xml |  2 +-
.../qemucapabilitiesdata/caps_2.10.0.ppc64.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.10.0.s390x.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.10.0.x86_64.xml |  2 +-
.../qemucapabilitiesdata/caps_2.11.0.s390x.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.11.0.x86_64.xml |  2 +-
.../caps_2.12.0.aarch64.xml |  2 +-
.../qemucapabilitiesdata/caps_2.12.0.ppc64.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.12.0.s390x.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.12.0.x86_64.xml |  2 +-
.../qemucapabilitiesdata/caps_2.4.0.x86_64.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.5.0.x86_64.xml  |  2 +-
.../qemucapabilitiesdata/caps_2.6.0.aarch64.xml |  2 +-
tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  2 +-
.../qemucapabilitiesdata/caps_2.6.0.x86_64.xml  |  2 +-
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml |  2 +-
.../qemucapabilitiesdata/caps_2.7.0.x86_64.xml  |  2 +-
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml |  2 +-
.../qemucapabilitiesdata/caps_2.8.0.x86_64.xml  |  2 +-
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  2 +-
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml |  2 +-
.../qemucapabilitiesdata/caps_2.9.0.x86_64.xml  |  2 +-
tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  2 +-
tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml |  2 +-
.../qemucapabilitiesdata/caps_3.0.0.x86_64.xml  |  2 +-
tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml |  2 +-
.../qemucapabilitiesdata/caps_3.1.0.x86_64.xml  |  2 +-
.../qemucapabilitiesdata/caps_4.0.0.x86_64.xml  |  2 +-
tests/qemucapabilitiestest.c| 17 +
33 files changed, 45 insertions(+), 36 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 09/15] storage: Use VIR_AUTOFREE for storage util

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:41AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---

[snip]

> @@ -3804,7 +3713,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>  {
>  virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
>  VIR_AUTOPTR(virStorageVolDef) vol = NULL;
> -char *devpath = NULL;
> +VIR_AUTOFREE(char *) devpath = NULL;
>  int retval = -1;
>
>  /* Check if the pool is using a stable target path. The call to
> @@ -3820,11 +3729,11 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>  virReportError(VIR_ERR_INVALID_ARG,
> _("unable to use target path '%s' for dev '%s'"),
> NULLSTR(def->target.path), dev);
> -goto cleanup;
> +return -1;
>  }
>
>  if (VIR_ALLOC(vol) < 0)
> -goto cleanup;
> +return -1;
>
>  vol->type = VIR_STORAGE_VOL_BLOCK;
>
> @@ -3834,10 +3743,10 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>   * just leave 'host' out
>   */
>  if (virAsprintf(&(vol->name), "unit:%u:%u:%u", bus, target, lun) < 0)
> -goto cleanup;
> +return -1;
>
>  if (virAsprintf(&devpath, "/dev/%s", dev) < 0)
> -goto cleanup;
> +return -1;
>
>  VIR_DEBUG("Trying to create volume for '%s'", devpath);
>
> @@ -3850,7 +3759,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>  if ((vol->target.path = virStorageBackendStablePath(pool,
>  devpath,
>  true)) == NULL)
> -goto cleanup;
> +return -1;
>
>  if (STREQ(devpath, vol->target.path) &&
>  !(STREQ(def->target.path, "/dev") ||
> @@ -3859,34 +3768,29 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>  VIR_DEBUG("No stable path found for '%s' in '%s'",
>devpath, def->target.path);
>
> -retval = -2;
> -goto cleanup;
> +return -2;
>  }
>
>  /* Allow a volume read failure to ignore or skip this block file */
>  if ((retval = virStorageBackendUpdateVolInfo(vol, true,
>   
> VIR_STORAGE_VOL_OPEN_DEFAULT,
>   
> VIR_STORAGE_VOL_READ_NOERROR)) < 0)
> -goto cleanup;
> +return retval;
>
>  vol->key = virStorageBackendSCSISerial(vol->target.path,
> (def->source.adapter.type ==
>  
> VIR_STORAGE_ADAPTER_TYPE_FC_HOST));
>  if (!vol->key)
> -goto cleanup;

We're changing the logic ^here - previously if virStorageBackendUpdateVolInfo
succeeded but virStorageBackendSCSISerial returned NULL, we'd still return
retval which would have been equal to 0. To me, your change feels right, but I
want to make sure no caller was relying on 0 even though
virStorageBackendSCSISerial might have failed.

Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH 03/15] conf: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageVolDef

2019-02-07 Thread John Ferlan



On 2/7/19 8:06 AM, Erik Skultety wrote:
> On Wed, Feb 06, 2019 at 08:41:35AM -0500, John Ferlan wrote:
>> Let's make use of the auto __cleanup capabilities cleaning up any
>> now unnecessary goto paths. For virStorageVolDefParseXML use the
>> @def/@ret similarly as other methods.
>>
>> Signed-off-by: John Ferlan 
>> ---
> ...
> 
>> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
>> index 6099c64b26..83ca379217 100644
>> --- a/src/conf/storage_conf.c
>> +++ b/src/conf/storage_conf.c
>> @@ -1168,7 +1168,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
>>   xmlXPathContextPtr ctxt,
>>   unsigned int flags)
>>  {
>> -virStorageVolDefPtr ret;
>> +VIR_AUTOPTR(virStorageVolDef) def = NULL;
>> +virStorageVolDefPtr ret = NULL;
>>  virStorageVolOptionsPtr options;
>>  char *type = NULL;
>>  char *allocation = NULL;
>> @@ -1187,132 +1188,132 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
>>  if (options == NULL)
>>  return NULL;
>>
>> -if (VIR_ALLOC(ret) < 0)
>> +if (VIR_ALLOC(def) < 0)
>>  return NULL;
>>
>> -ret->target.type = VIR_STORAGE_TYPE_FILE;
>> +def->target.type = VIR_STORAGE_TYPE_FILE;
>>
>> -ret->name = virXPathString("string(./name)", ctxt);
>> -if (ret->name == NULL) {
>> +def->name = virXPathString("string(./name)", ctxt);
>> +if (def->name == NULL) {
>>  virReportError(VIR_ERR_XML_ERROR, "%s",
>> _("missing volume name element"));
>> -goto error;
>> +goto cleanup;
>>  }
>>
>>  /* Normally generated by pool refresh, but useful for unit tests */
>> -ret->key = virXPathString("string(./key)", ctxt);
>> +def->key = virXPathString("string(./key)", ctxt);
>>
>>  /* Technically overridden by pool refresh, but useful for unit tests */
>>  type = virXPathString("string(./@type)", ctxt);
>>  if (type) {
>> -if ((ret->type = virStorageVolTypeFromString(type)) < 0) {
>> +if ((def->type = virStorageVolTypeFromString(type)) < 0) {
>>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("unknown volume type '%s'"), type);
>> -goto error;
>> +goto cleanup;
>>  }
>>  }
>>
>>  if ((backingStore = virXPathString("string(./backingStore/path)", 
>> ctxt))) {
>> -if (VIR_ALLOC(ret->target.backingStore) < 0)
>> -goto error;
>> +if (VIR_ALLOC(def->target.backingStore) < 0)
>> +goto cleanup;
>>
>> -ret->target.backingStore->type = VIR_STORAGE_TYPE_FILE;
>> +def->target.backingStore->type = VIR_STORAGE_TYPE_FILE;
>>
>> -ret->target.backingStore->path = backingStore;
>> +def->target.backingStore->path = backingStore;
>>  backingStore = NULL;
>>
>>  if (options->formatFromString) {
>>  char *format = 
>> virXPathString("string(./backingStore/format/@type)", ctxt);
>>  if (format == NULL)
>> -ret->target.backingStore->format = options->defaultFormat;
>> +def->target.backingStore->format = options->defaultFormat;
>>  else
>> -ret->target.backingStore->format = 
>> (options->formatFromString)(format);
>> +def->target.backingStore->format = 
>> (options->formatFromString)(format);
>>
>> -if (ret->target.backingStore->format < 0) {
>> +if (def->target.backingStore->format < 0) {
>>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("unknown volume format type %s"), format);
>>  VIR_FREE(format);
>> -goto error;
>> +goto cleanup;
>>  }
>>  VIR_FREE(format);
>>  }
>>
>> -if (VIR_ALLOC(ret->target.backingStore->perms) < 0)
>> -goto error;
>> -if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms,
>> +if (VIR_ALLOC(def->target.backingStore->perms) < 0)
>> +goto cleanup;
>> +if (virStorageDefParsePerms(ctxt, def->target.backingStore->perms,
>>  "./backingStore/permissions") < 0)
>> -goto error;
>> +goto cleanup;
>>  }
>>
>>  capacity = virXPathString("string(./capacity)", ctxt);
>>  unit = virXPathString("string(./capacity/@unit)", ctxt);
>>  if (capacity) {
>> -if (virStorageSize(unit, capacity, &ret->target.capacity) < 0)
>> -goto error;
>> +if (virStorageSize(unit, capacity, &def->target.capacity) < 0)
>> +goto cleanup;
>>  } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) &&
>> !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) &&
>> - virStorageSourceHasBacking(&ret->target))) {
>> + virStorageSourceHasBacking(&def->target))) {
>>  virReportError(VI

Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

2019-02-07 Thread Ján Tomko

On Thu, Feb 07, 2019 at 07:12:08AM -0500, John Ferlan wrote:



On 2/7/19 4:10 AM, Erik Skultety wrote:

On Wed, Feb 06, 2019 at 08:41:33AM -0500, John Ferlan wrote:

Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths. For virStorageAuthDefCopy use authdef
and ret as consistently as similar other code.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c| 29 +++--
 src/conf/storage_conf.c   |  6 ++
 src/qemu/qemu_parse_command.c |  6 ++
 src/util/virstoragefile.c | 33 ++---
 src/util/virstoragefile.h |  1 +
 5 files changed, 30 insertions(+), 45 deletions(-)




+VIR_STEAL_PTR(iscsisrc->src->auth, authdef);


^Unrelated change, there should be a trivial patch preceding this one taking
care of the VIR_STEAL_PTR replacements.

...


You'll find this sprinkled throughout - generating separate patches
could explode the series into perhaps 30+ patches which I doubt anyone
would jump at the chance to review.  I can separate before pushing and I
assume by extracting it/them I can just add your R-by too...



Splitting the changes into simple patches doing one thing at a time
actually makes review easier, even though it makes the series longer.

Jano


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

Re: [libvirt] [PATCH 08/15] storage: Use VIR_AUTOFREE for storage driver

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:40AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH 07/15] storage: Use VIR_AUTOFREE for storage backends

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:39AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities. This also allows
> for the cleanup of some goto paths.
>
> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend.c  |  9 +--
>  src/storage/storage_backend_disk.c | 62 +++---
>  src/storage/storage_backend_fs.c   | 17 ++---
>  src/storage/storage_backend_gluster.c  | 30 -
>  src/storage/storage_backend_iscsi.c| 73 +++---
>  src/storage/storage_backend_iscsi_direct.c | 37 ---
>  src/storage/storage_backend_logical.c  | 35 ---
>  src/storage/storage_backend_mpath.c| 18 +++---
>  src/storage/storage_backend_rbd.c  | 35 ---
>  src/storage/storage_backend_scsi.c | 63 +++
>  src/storage/storage_backend_sheepdog.c | 27 +++-
>  src/storage/storage_backend_vstorage.c | 25 +++-
>  src/storage/storage_backend_zfs.c  | 15 ++---
>  src/storage/storage_file_gluster.c | 16 ++---
>  14 files changed, 153 insertions(+), 309 deletions(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index a54c338cf0..5c8275e978 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -87,8 +87,7 @@ virStorageDriverLoadBackendModule(const char *name,
>const char *regfunc,
>bool forceload)
>  {
> -char *modfile = NULL;
> -int ret;
> +VIR_AUTOFREE(char *) modfile = NULL;
>
>  if (!(modfile = virFileFindResourceFull(name,
>  "libvirt_storage_backend_",
> @@ -98,11 +97,7 @@ virStorageDriverLoadBackendModule(const char *name,
>  "LIBVIRT_STORAGE_BACKEND_DIR")))
>  return -1;
>
> -ret = virModuleLoad(modfile, regfunc, forceload);
> -
> -VIR_FREE(modfile);
> -
> -return ret;
> +return virModuleLoad(modfile, regfunc, forceload);
>  }
>
>
> diff --git a/src/storage/storage_backend_disk.c 
> b/src/storage/storage_backend_disk.c
> index 230cf44b97..f2f56ee3de 100644
> --- a/src/storage/storage_backend_disk.c
> +++ b/src/storage/storage_backend_disk.c
> @@ -56,7 +56,8 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
>   virStorageVolDefPtr vol)
>  {
>  virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> -char *tmp, *devpath, *partname;
> +char *tmp, *partname;
> +VIR_AUTOFREE(char *) devpath = NULL;
>  bool addVol = false;
>
>  /* Prepended path will be same for all partitions, so we can
> @@ -89,7 +90,6 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
>   * way of doing this...
>   */
>  vol->target.path = virStorageBackendStablePath(pool, devpath, true);
> -VIR_FREE(devpath);
>  if (vol->target.path == NULL)
>  goto error;
>  }
> @@ -355,13 +355,12 @@ 
> virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool,
>   */
>
>  virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> -char *parthelper_path;
> +VIR_AUTOFREE(char *) parthelper_path = NULL;
>  VIR_AUTOPTR(virCommand) cmd = NULL;
>  struct virStorageBackendDiskPoolVolData cbdata = {
>  .pool = pool,
>  .vol = vol,
>  };
> -int ret;
>
>  if (!(parthelper_path = virFileFindResource("libvirt_parthelper",
>  abs_topbuilddir "/src",
> @@ -388,12 +387,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr 
> pool,
>  def->allocation = 0;
>  def->capacity = def->available = 0;
>
> -ret = virCommandRunNul(cmd,
> -   6,
> -   virStorageBackendDiskMakeVol,
> -   &cbdata);
> -VIR_FREE(parthelper_path);
> -return ret;
> +return virCommandRunNul(cmd, 6, virStorageBackendDiskMakeVol, &cbdata);
>  }
>
>  static int
> @@ -419,9 +413,8 @@ static int
>  virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool)
>  {
>  virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> -char *parthelper_path;
> +VIR_AUTOFREE(char *) parthelper_path = NULL;
>  VIR_AUTOPTR(virCommand) cmd = NULL;
> -int ret;
>
>  if (!(parthelper_path = virFileFindResource("libvirt_parthelper",
>  abs_topbuilddir "/src",
> @@ -433,12 +426,8 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr 
> pool)
> "-g",
> NULL);
>
> -ret = virCommandRunNul(cmd,
> -   3,
> -   virStorageBackendDiskMakePoolGeometry,
> -   pool);
> -VIR_FREE(parthelper_path);
> -return ret;
> +return virCommandRunNul(cmd, 3, virS

Re: [libvirt] [PATCH 8/8] virsh: Add support for setting post-copy migration bandwidth

2019-02-07 Thread Ján Tomko

On Tue, Feb 05, 2019 at 04:23:11PM +0100, Jiri Denemark wrote:

Signed-off-by: Jiri Denemark 
---
tools/virsh-domain.c | 33 +++--
tools/virsh.pod  | 15 +++
2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 275ac0c318..db0d5d4dcc 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11150,6 +11163,10 @@ static const vshCmdOptDef opts_migrate_setspeed[] = {
 .flags = VSH_OFLAG_REQ,
 .help = N_("migration bandwidth limit in MiB/s")
},
+{.name = "postcopy",
+ .type = VSH_OT_BOOL,
+ .help = N_("set postcopy migration bandwidth")


post-copy


+},
{.name = NULL}
};

@@ -11191,6 +11212,10 @@ static const vshCmdInfo info_migrate_getspeed[] = {

static const vshCmdOptDef opts_migrate_getspeed[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
+{.name = "postcopy",
+ .type = VSH_OT_BOOL,
+ .help = N_("get postcopy migration bandwidth")


post-copy


+},
{.name = NULL}
};



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 7/8] qemu: Implement VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag

2019-02-07 Thread Ján Tomko

On Tue, Feb 05, 2019 at 04:23:10PM +0100, Jiri Denemark wrote:

This flag tells virDomainMigrateSetMaxSpeed and
virDomainMigrateGetMaxSpeed APIs to work on post-copy migration
bandwidth.

Signed-off-by: Jiri Denemark 
---
src/qemu/qemu_driver.c | 91 --
1 file changed, 78 insertions(+), 13 deletions(-)





@@ -14344,7 +14369,47 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom,
if (virDomainMigrateGetMaxSpeedEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;

-*bandwidth = priv->migMaxBandwidth;
+if (postcopy) {


This whole branch looks like a good candidate for a helper function.


+VIR_AUTOPTR(qemuMigrationParams) migParams = NULL;
+unsigned long long bw;
+int rc = -1;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) == 0 &&
+qemuMigrationParamsFetch(driver, vm, QEMU_ASYNC_JOB_NONE,
+ &migParams) == 0) {
+rc = qemuMigrationParamsGetULL(migParams,
+   
QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH,
+   &bw);
+
+/* QEMU reports B/s while we use MiB/s */
+bw /= 1024 * 1024;
+}


You coould put an 'endjob' label here.


+
+qemuDomainObjEndJob(driver, vm);
+
+if (rc < 0) {
+goto cleanup;
+} else if (rc == 1) {


No need for 'else' after goto.


+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("querying maximum post-copy migration speed is "
+ "not supported by QEMU binary"));
+goto cleanup;
+} if (bw > ULONG_MAX) {


Missing newline.

Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 0/3] bhyve: implement MSRs ignore unknown writes feature

2019-02-07 Thread Roman Bogorodskiy
  Daniel P. Berrangé wrote:

> On Wed, Feb 06, 2019 at 04:00:06PM -0500, Cole Robinson wrote:
> > On 1/25/19 12:54 PM, Roman Bogorodskiy wrote:
> > > 
> > > Roman Bogorodskiy (3):
> > >   conf: introduce 'msrs' feature
> > >   bhyve: implement MSRs ignore unknown writes feature
> > >   news: document bhyve msrs feature
> > > 
> > >  docs/drvbhyve.html.in | 16 +
> > >  docs/formatdomain.html.in |  1 +
> > >  docs/news.xml | 11 ++
> > >  docs/schemas/domaincommon.rng | 14 
> > >  src/bhyve/bhyve_command.c |  4 +++
> > >  src/conf/domain_conf.c| 33 +
> > >  src/conf/domain_conf.h|  8 +
> > >  src/qemu/qemu_domain.c|  1 +
> > >  .../bhyvexml2argvdata/bhyvexml2argv-msrs.args | 10 ++
> > >  .../bhyvexml2argv-msrs.ldargs |  3 ++
> > >  .../bhyvexml2argvdata/bhyvexml2argv-msrs.xml  | 26 ++
> > >  tests/bhyvexml2argvtest.c |  1 +
> > >  .../bhyvexml2xmlout-msrs.xml  | 36 +++
> > >  tests/bhyvexml2xmltest.c  |  1 +
> > >  14 files changed, 165 insertions(+)
> > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.args
> > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.ldargs
> > >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-msrs.xml
> > >  create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-msrs.xml
> > > 
> > 
> > The code looks fine to me but this needs a bit more context.
> > Particularly 'ignore' is a bit ambiguous here so I had to dig
> > 
> > kvm+linux has arguably always ignored unknown MSRs but historically it
> > would print an error in host dmesg which often confused users quite a
> > bit. In the bhyve case it seems that unknown MSRs cause the VM to
> > essentially crash which is a pretty intense reaction. This option
> > disables that crashing behavior. So for this feature to be really
> > descriptive it would be  or something
> > like that
> 
> That is not actually the case for KVM.   When KVM sees an unhandled
> MSR it will inject a GPF to the guest, which will usually cause the
> guest to crash. KVM does this GPF injection for both reads and writes
> of unknown MSRs.
> 
> There is an opt-in kvm.ko  option "ignore-msrs" which tells KVM
> to silently ignore unknown MSRs instead. Ideally the KVM option
> would be exposed to QEMU as an option, since when people do need
> it, they usually only need it for a single guest.
> 
> > The bhyve man page says:
> > 
> > https://www.freebsd.org/cgi/man.cgi?query=bhyve&sektion=8
> > 
> > -wIgnore accesses to unimplemented Model Specific Registers
> >   (MSRs). This is intended for debug purposes.
> 
> I'm curious whether this applies to boths reads & writes of unknown
> MSRs, or just to writes.  '-w' naming suggests the latter, but the
> descrption "accesses" would suggest reads too.

From what I can see by briefly looking at the code, it applies to both
reads and writes:

https://svnweb.freebsd.org/base/head/usr.sbin/bhyve/bhyverun.c?revision=343642&view=markup#l520

Check two functions below this line.

> > 
> > Calling it 'intended for debug purposes' also makes me question whether
> > this should be encoded in the libvirt XML or just worked around with
> > commandline passthrough
> > 
> > (IMO To be friendly to users this option should be enabled by default
> > but obviously that's against the intention of bhyve devs...)
> 
> Enabling it by default is not a good idea as it can lead to silently
> incorrect guest OS behaviour. That could lead to all manner of bad
> things depending on the MSR in question.
> 
> > ccing danpb to see if he has any thoughs about exposing this in the XML
> 
> My only thought is the naming & whether we have one attribute for
> ignoring reads & writes, or separate attributes.
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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 06/15] storage: Use VIR_AUTOPTR(virCommand)

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:38AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH 05/15] storage: Use VIR_AUTOPTR(virString)

2019-02-07 Thread Erik Skultety
On Wed, Feb 06, 2019 at 08:41:37AM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities cleaning up any
> now unnecessary goto paths.
>
> Signed-off-by: John Ferlan 
> ---
Reviewed-by: Erik Skultety 

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


Re: [libvirt] [PATCH 6/8] qemu: Implement VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY

2019-02-07 Thread Ján Tomko

On Tue, Feb 05, 2019 at 04:23:09PM +0100, Jiri Denemark wrote:

This typed parameter for virDomainMigrate3 and virDomainMigrateToURI3
APIs may be used for setting maximum post-copy migration bandwidth.

Signed-off-by: Jiri Denemark 
---
src/qemu/qemu_migration.h| 1 +
src/qemu/qemu_migration_params.c | 7 +++
src/qemu/qemu_migration_params.h | 1 +
3 files changed, 9 insertions(+)




diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 117ae0e998..67070b9d08 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -178,6 +179,11 @@ static const qemuMigrationParamsTPMapItem 
qemuMigrationParamsTPMap[] = {
{.typedParam = VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE,
 .param = QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE,
 .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION},
+
+{.typedParam = VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY,
+ .unit = 1024 * 1024, /* MB/s */


MiB/s


+ .param = QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH,
+ .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION},
};

static const qemuMigrationParamType qemuMigrationParamTypes[] = {


Reviewed-by: Ján Tomko 

Jano


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

[libvirt] [PATCH 5/5] conf: Remove volOptions for VIR_STORAGE_POOL_MPATH

2019-02-07 Thread John Ferlan
The multipath pool is documented as not using the volume type,
so let's just remove it.

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 264e5827b0..e9c943d25c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -250,9 +250,6 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
  }
 },
 {.poolType = VIR_STORAGE_POOL_MPATH,
- .volOptions = {
- .formatToString = virStoragePoolFormatDiskTypeToString,
- }
 },
 {.poolType = VIR_STORAGE_POOL_DISK,
  .poolOptions = {
-- 
2.20.1

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


Re: [libvirt] [PATCH 5/8] Public API for post-copy migration bandwidth

2019-02-07 Thread Ján Tomko

On Tue, Feb 05, 2019 at 04:23:08PM +0100, Jiri Denemark wrote:

This patch adds a new VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY typed
parameter for virDomainMigrate3 and virDomainMigrateToURI3 for setting
maximum post-copy migration bandwidth.

In case the initial VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY value turns out
to be suboptimal a new VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag for
virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed may be used
to set/get the maximum post-copy migration bandwidth while migration is
already running.

Signed-off-by: Jiri Denemark 
---
include/libvirt/libvirt-domain.h | 15 +++
src/libvirt-domain.c | 11 ---
2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 0388911ded..d99f9b690b 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -903,6 +903,15 @@ typedef enum {
 */
# define VIR_MIGRATE_PARAM_BANDWIDTH "bandwidth"

+/**
+ * VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY:
+ *
+ * virDomainMigrate* params field: the maximum bandwidth (in MiB/s) that will


I guess granularity < 1 MiB/s is not worth the trouble of creating an
inconsistent API, is it?


+ * be used for post-copy phase of a migration as VIR_TYPED_PARAM_ULLONG. If set
+ * to 0 or omitted, post-copy migration speed will not be limited.
+ */
+# define VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY "bandwidth.postcopy"
+
/**
 * VIR_MIGRATE_PARAM_GRAPHICS_URI:
 *
@@ -1062,6 +1071,12 @@ int virDomainMigrateSetCompressionCache(virDomainPtr 
domain,
unsigned long long cacheSize,
unsigned int flags);

+/* Domain migration speed flags. */
+typedef enum {
+/* Set or get maximum speed of postcopy migration. */


post-copy is the spelling used for human-readable strings


+VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY = (1 << 0),
+} virDomainMigrateMaxSpeedFlags;
+
int virDomainMigrateSetMaxSpeed(virDomainPtr domain,
unsigned long bandwidth,
unsigned int flags);
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 58b4863c8f..7b87b3716a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9036,11 +9036,13 @@ virDomainMigrateSetCompressionCache(virDomainPtr domain,
 * virDomainMigrateSetMaxSpeed:
 * @domain: a domain object
 * @bandwidth: migration bandwidth limit in MiB/s
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainMigrateMaxSpeedFlags
 *
 * The maximum bandwidth (in MiB/s) that will be used to do migration
 * can be specified with the bandwidth parameter. Not all hypervisors
- * will support a bandwidth cap
+ * will support a bandwidth cap. When VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY
+ * is set in @flags, this API sets the maximum bandwidth for the postcopy


post-copy


+ * phase of the migration.
 *
 * Returns 0 in case of success, -1 otherwise.
 */
@@ -9077,10 +9079,13 @@ virDomainMigrateSetMaxSpeed(virDomainPtr domain,
 * virDomainMigrateGetMaxSpeed:
 * @domain: a domain object
 * @bandwidth: return value of current migration bandwidth limit in MiB/s
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainMigrateMaxSpeedFlags
 *
 * Get the current maximum bandwidth (in MiB/s) that will be used if the
 * domain is migrated.  Not all hypervisors will support a bandwidth limit.
+ * When VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY is set in @flags, this API
+ * gets the current maximum bandwidth for the postcopy phase of the


post-copy


+ * migration.
 *


Reviewed-by: Ján Tomko 

Jano


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

[libvirt] [PATCH 2/5] conf: Remove volOptions for VIR_STORAGE_POOL_RBD

2019-02-07 Thread John Ferlan
The rbd pool is documented as not using the volume type,
so let's just remove it.

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index ac7cc77e82..8e87e79019 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -237,11 +237,6 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
VIR_STORAGE_POOL_SOURCE_NETWORK |
VIR_STORAGE_POOL_SOURCE_NAME),
   },
-  .volOptions = {
-  .defaultFormat = VIR_STORAGE_FILE_RAW,
-  .formatFromString = virStorageVolumeFormatFromString,
-  .formatToString = virStorageFileFormatTypeToString,
-  }
 },
 {.poolType = VIR_STORAGE_POOL_SHEEPDOG,
  .poolOptions = {
-- 
2.20.1

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


[libvirt] [PATCH 0/5] Remove some volOptions from virStoragePoolTypeInfo

2019-02-07 Thread John Ferlan
While working through changes to add storage pool capabilities
I found that the volOptions definitions for a few pool types were
not necessary and in a couple of cases wrong. Figured I would
extract them out of my work on those adjustments while I'm still
working through the code.

John Ferlan (5):
  conf: Remove volOptions for VIR_STORAGE_POOL_SHEEPDOG
  conf: Remove volOptions for VIR_STORAGE_POOL_RBD
  conf: Remove volOptions for VIR_STORAGE_POOL_SCSI
  conf: Remove volOptions for VIR_STORAGE_POOL_ISCSI[_DIRECT]
  conf: Remove volOptions for VIR_STORAGE_POOL_MPATH

 src/conf/storage_conf.c | 21 -
 tests/storagevolxml2xmlout/vol-sheepdog.xml |  1 -
 2 files changed, 22 deletions(-)

-- 
2.20.1

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


[libvirt] [PATCH 1/5] conf: Remove volOptions for VIR_STORAGE_POOL_SHEEPDOG

2019-02-07 Thread John Ferlan
The sheepdog pool is documented as not using the volume type,
so let's just remove it.  Besides it would have produced bad
results since the defaultType is FILE but the formatting used
the Disk types.

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.c | 4 
 tests/storagevolxml2xmlout/vol-sheepdog.xml | 1 -
 2 files changed, 5 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 1ee31ca676..ac7cc77e82 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -249,10 +249,6 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
VIR_STORAGE_POOL_SOURCE_NETWORK |
VIR_STORAGE_POOL_SOURCE_NAME),
  },
- .volOptions = {
- .defaultFormat = VIR_STORAGE_FILE_RAW,
- .formatToString = virStoragePoolFormatDiskTypeToString,
- }
 },
 {.poolType = VIR_STORAGE_POOL_GLUSTER,
  .poolOptions = {
diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml 
b/tests/storagevolxml2xmlout/vol-sheepdog.xml
index e1d6a9e27d..d6e920bb81 100644
--- a/tests/storagevolxml2xmlout/vol-sheepdog.xml
+++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml
@@ -6,6 +6,5 @@
   0
   
 sheepdog:test2
-
   
 
-- 
2.20.1

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


[libvirt] [PATCH 3/5] conf: Remove volOptions for VIR_STORAGE_POOL_SCSI

2019-02-07 Thread John Ferlan
The scsi pool is documented as not using the volume type,
so let's just remove it.

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 8e87e79019..085501b118 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -227,9 +227,6 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
  .poolOptions = {
  .flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER),
  },
- .volOptions = {
- .formatToString = virStoragePoolFormatDiskTypeToString,
- }
 },
 {.poolType = VIR_STORAGE_POOL_RBD,
  .poolOptions = {
-- 
2.20.1

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


[libvirt] [PATCH 4/5] conf: Remove volOptions for VIR_STORAGE_POOL_ISCSI[_DIRECT]

2019-02-07 Thread John Ferlan
The iscsi and iscsi-direct pools are documented as not using
the volume type, so let's just remove it. Besides it would
have produced bad output since formatting uses the Disk types.

Signed-off-by: John Ferlan 
---
 src/conf/storage_conf.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 085501b118..264e5827b0 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -208,9 +208,6 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
VIR_STORAGE_POOL_SOURCE_DEVICE |
VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN),
   },
-  .volOptions = {
- .formatToString = virStoragePoolFormatDiskTypeToString,
-  }
 },
 {.poolType = VIR_STORAGE_POOL_ISCSI_DIRECT,
  .poolOptions = {
@@ -219,9 +216,6 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
VIR_STORAGE_POOL_SOURCE_NETWORK |
VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN),
   },
-  .volOptions = {
- .formatToString = virStoragePoolFormatDiskTypeToString,
-  }
 },
 {.poolType = VIR_STORAGE_POOL_SCSI,
  .poolOptions = {
-- 
2.20.1

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


Re: [libvirt] [PATCH 3/8] qemu: Rework qemuDomainMigrateSetMaxSpeed

2019-02-07 Thread Ján Tomko

On Tue, Feb 05, 2019 at 04:23:06PM +0100, Jiri Denemark wrote:

Let's make the code flow easier to follow and get rid of the ugly endjob
label inside if branch.



Thank you.


Signed-off-by: Jiri Denemark 
---
src/qemu/qemu_driver.c | 41 ++---
1 file changed, 22 insertions(+), 19 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 4/8] qemu: Make migration params usable outside migration

2019-02-07 Thread Ján Tomko

On Tue, Feb 05, 2019 at 04:23:07PM +0100, Jiri Denemark wrote:

So far migration parameters were changed only at the beginning of
migration mostly via an automatic translation from flags and typed
parameters. We need to export a few more functions to support APIs which
may set migration parameters while migration is already running.

Signed-off-by: Jiri Denemark 
---
src/qemu/qemu_migration_params.c | 16 +++-
src/qemu/qemu_migration_params.h |  9 +
2 files changed, 24 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


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

  1   2   >