date:20191217

Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci

2019-12-17 Thread Jason Wang



On 2019/12/12 下午1:47, Yan Zhao wrote:

On Thu, Dec 12, 2019 at 11:48:25AM +0800, Jason Wang wrote:

On 2019/12/6 下午8:49, Yan Zhao wrote:

On Fri, Dec 06, 2019 at 05:40:02PM +0800, Jason Wang wrote:

On 2019/12/6 下午4:22, Yan Zhao wrote:

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:

On 2019/12/5 下午4:51, Yan Zhao wrote:

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:

Hi:

On 2019/12/5 上午11:24, Yan Zhao wrote:

For SRIOV devices, VFs are passthroughed into guest directly without host
driver mediation. However, when VMs migrating with passthroughed VFs,
dynamic host mediation is required to  (1) get device states, (2) get
dirty pages. Since device states as well as other critical information
required for dirty page tracking for VFs are usually retrieved from PFs,
it is handy to provide an extension in PF driver to centralizingly control
VFs' migration.

Therefore, in order to realize (1) passthrough VFs at normal time, (2)
dynamically trap VFs' bars for dirty page tracking and

A silly question, what's the reason for doing this, is this a must for dirty
page tracking?


For performance consideration. VFs' bars should be passthoughed at
normal time and only enter into trap state on need.

Right, but how does this matter for the case of dirty page tracking?


Take NIC as an example, to trap its VF dirty pages, software way is
required to trap every write of ring tail that resides in BAR0.

Interesting, but it looks like we need:
- decode the instruction
- mediate all access to BAR0
All of which seems a great burden for the VF driver. I wonder whether or
not doing interrupt relay and tracking head is better in this case.


hi Jason

not familiar with the way you mentioned. could you elaborate more?


It looks to me that you want to intercept the bar that contains the
head. Then you can figure out the buffers submitted from driver and you
still need to decide a proper time to mark them as dirty.


Not need to be accurate, right? just a superset of real dirty bitmap is
enough.



If the superset is too large compared with the dirty pages, it will lead 
a lot of side effects.






What I meant is, intercept the interrupt, then you can figure still
figure out the buffers which has been modified by the device and make
them as dirty.

Then there's no need to trap BAR and do decoding/emulation etc.

But it will still be tricky to be correct...


intercept the interrupt is a little hard if post interrupt is enabled..



We don't care about the performance too much in this case. Can we simply 
disable it?




I think what you worried about here is the timing to mark dirty pages,
right? upon interrupt receiving, you regard DMAs are finished and safe
to make them dirty.
But with BAR trap way, we at least can keep those dirtied pages as dirty
until device stop. Of course we have other methods to optimize it.



I'm not sure this will not confuse the migration converge time estimation.





There's
still no IOMMU Dirty bit available.

  (3) centralizing
VF critical states retrieving and VF controls into one driver, we propose
to introduce mediate ops on top of current vfio-pci device driver.


_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
  __   register mediate ops|  ___ ___|
|  |<---| VF|   |   |
| vfio-pci |  | |  mediate  |   | PF driver |   |
|__|--->|   driver  |   |___|
  |open(pdev)  |  ---  | |
  ||
  ||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _|
 \|/  \|/
--- 
|VF   | |PF|
--- 


VF mediate driver could be a standalone driver that does not bind to
any devices (as in demo code in patches 5-6) or it could be a built-in
extension of PF driver (as in patches 7-9) .

Rather than directly bind to VF, VF mediate driver register a mediate
ops into vfio-pci in driver init. vfio-pci maintains a list of such
mediate ops.
(Note that: VF mediate driver can register mediate ops into vfio-pci
before vfio-pci binding to any devices. And VF mediate driver can
support mediating multiple devices.)

When opening a device (e.g. a VF), vfio-pci goes through the mediate ops
list and calls each vfio_pci_mediate_ops->open() with pdev of the opening
device as a parameter.
VF mediate driver should return success or failure depending on it
supports the pdev or not.
E.g. VF mediate driver would compare its supported VF devfn with the
devfn of the passed-in pdev.
Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will
stop querying other mediate ops and bind the opening devi

[libvirt] [PATCH v2 1/4] qemu: command: move NVDIMM validation to qemu_domain.c

2019-12-17 Thread Daniel Henrique Barboza

Move the NVDIMM validation from qemuBuildMachineCommandLine()
to a new function in qemu_domain.c, qemuDomainDeviceDefValidateMemory(),
which is called by qemuDomainDeviceDefValidate(). This allows
NVDIMM validation to occur in domain define time.

It also increments memory hotplug validation, which can be seen
by the failures in the hotplug tests in qemuxml2xmltest.c that
needed to be adjusted after the move.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_command.c |  5 -
 src/qemu/qemu_domain.c  | 18 +-
 tests/qemuxml2xmltest.c | 12 ++--
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 836057a4ff..3267b0d4b5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6977,11 +6977,6 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 
 for (i = 0; i < def->nmems; i++) {
 if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("nvdimm isn't supported by this QEMU 
binary"));
-return -1;
-}
 virBufferAddLit(&buf, ",nvdimm=on");
 break;
 }
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 20a9629760..83964db595 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5637,6 +5637,19 @@ qemuDomainDeviceDefValidateSound(virDomainSoundDefPtr 
sound,
 return 0;
 }
 
+static int
+qemuDomainDeviceDefValidateMemory(virDomainMemoryDefPtr mem,
+  virQEMUCapsPtr qemuCaps)
+{
+if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("nvdimm isn't supported by this QEMU binary"));
+return -1;
+}
+
+return 0;
+}
 
 static int
 qemuDomainDefValidate(const virDomainDef *def,
@@ -8365,9 +8378,12 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef 
*dev,
 ret = qemuDomainDeviceDefValidateSound(dev->data.sound, qemuCaps);
 break;
 
+case VIR_DOMAIN_DEVICE_MEMORY:
+ret = qemuDomainDeviceDefValidateMemory(dev->data.memory, qemuCaps);
+break;
+
 case VIR_DOMAIN_DEVICE_LEASE:
 case VIR_DOMAIN_DEVICE_SHMEM:
-case VIR_DOMAIN_DEVICE_MEMORY:
 case VIR_DOMAIN_DEVICE_PANIC:
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_LAST:
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 55bbd924fb..60ae68c58c 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1221,12 +1221,12 @@ mymain(void)
 DO_TEST("memory-hotplug", NONE);
 DO_TEST("memory-hotplug-nonuma", NONE);
 DO_TEST("memory-hotplug-dimm", NONE);
-DO_TEST("memory-hotplug-nvdimm", NONE);
-DO_TEST("memory-hotplug-nvdimm-access", NONE);
-DO_TEST("memory-hotplug-nvdimm-label", NONE);
-DO_TEST("memory-hotplug-nvdimm-align", NONE);
-DO_TEST("memory-hotplug-nvdimm-pmem", NONE);
-DO_TEST("memory-hotplug-nvdimm-readonly", NONE);
+DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_DEVICE_NVDIMM);
+DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_DEVICE_NVDIMM);
+DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM);
+DO_TEST("memory-hotplug-nvdimm-align", QEMU_CAPS_DEVICE_NVDIMM);
+DO_TEST("memory-hotplug-nvdimm-pmem", QEMU_CAPS_DEVICE_NVDIMM);
+DO_TEST("memory-hotplug-nvdimm-readonly", QEMU_CAPS_DEVICE_NVDIMM);
 DO_TEST("net-udp", NONE);
 
 DO_TEST("video-virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU);
-- 
2.23.0


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

[libvirt] [PATCH v2 3/4] qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c

2019-12-17 Thread Daniel Henrique Barboza

Move smartcard validation being done by qemuBuildSmartcardCommandLine()
to the existing qemuDomainSmartcardDefValidate() function. This
function is called by qemuDomainDeviceDefValidate(), allowing smartcard
validation in domain define time.

Tests were adapted to consider the new caps being needed in
this earlier stage.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_command.c | 24 
 src/qemu/qemu_domain.c  | 33 +
 tests/qemuxml2xmltest.c | 16 +---
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b3f069d5d4..67f7caf9c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8275,24 +8275,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
logManager,
 
 switch (smartcard->type) {
 case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("this QEMU binary lacks smartcard host "
- "mode support"));
-return -1;
-}
-
 virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated");
 break;
 
 case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("this QEMU binary lacks smartcard host "
- "mode support"));
-return -1;
-}
-
 virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates");
 for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
 virBufferAsprintf(&opt, ",cert%zu=", i + 1);
@@ -8308,13 +8294,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
logManager,
 break;
 
 case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("this QEMU binary lacks smartcard "
- "passthrough mode support"));
-return -1;
-}
-
 if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
   cmd, cfg, def,
   smartcard->data.passthru,
@@ -8330,9 +8309,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
 break;
 
 default:
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("unexpected smartcard type %d"),
-   smartcard->type);
 return -1;
 }
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fe353e5bc1..f6683d11e0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6156,6 +6156,39 @@ static int
 qemuDomainSmartcardDefValidate(const virDomainSmartcardDef *def,
virQEMUCapsPtr qemuCaps)
 {
+switch (def->type) {
+case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this QEMU binary lacks smartcard host "
+ "mode support"));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this QEMU binary lacks smartcard host "
+ "mode support"));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this QEMU binary lacks smartcard "
+ "passthrough mode support"));
+return -1;
+}
+break;
+
+default:
+virReportEnumRangeError(virDomainSmartcardType, def->type);
+return -1;
+}
+
 if (def->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH &&
 qemuDomainChrSourceDefValidate(def->data.passthru, qemuCaps) < 0)
 return -1;
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 60ae68c58c..64321bcb80 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1332,12 +1332,13 @@ mymain(void)
 DO_TEST("cpu-check-default-partial2", NONE);
 DO_TEST("vmcoreinfo", NONE);
 
-DO_TEST("smartcard-host", NONE);
-DO_TEST("smartcard-host-certificates", NONE);
-DO_TEST("smartcard-host-certificates-database", NONE);
-DO_TEST("smartcard-passthrough-tcp", NONE);
-DO_TEST("smartcard-passthrough-spicevmc", NONE);
-DO_TEST("smartcard-control

[libvirt] [PATCH v2 4/4] qemu: command: move validation of vmcoreinfo to qemu_domain.c

2019-12-17 Thread Daniel Henrique Barboza

Move the validation of vmcoreinfo from qemuBuildVMCoreInfoCommandLine()
to qemuDomainDefValidateFeatures(), allowing for validation
at domain define time.

qemuxml2xmltest.c was changed to account for this caps being
now validated at this earlier stage.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_command.c | 12 ++--
 src/qemu/qemu_domain.c  | 11 ++-
 tests/qemuxml2xmltest.c |  2 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 67f7caf9c6..44cc647359 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9217,21 +9217,13 @@ qemuBuildSEVCommandLine(virDomainObjPtr vm, 
virCommandPtr cmd,
 
 static int
 qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
-   const virDomainDef *def,
-   virQEMUCapsPtr qemuCaps)
+   const virDomainDef *def)
 {
 virTristateSwitch vmci = def->features[VIR_DOMAIN_FEATURE_VMCOREINFO];
 
 if (vmci != VIR_TRISTATE_SWITCH_ON)
 return 0;
 
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMCOREINFO)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("vmcoreinfo is not available "
- "with this QEMU binary"));
-return -1;
-}
-
 virCommandAddArgList(cmd, "-device", "vmcoreinfo", NULL);
 return 0;
 }
@@ -9933,7 +9925,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
 if (qemuBuildNVRAMCommandLine(cmd, def) < 0)
 return NULL;
 
-if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
+if (qemuBuildVMCoreInfoCommandLine(cmd, def) < 0)
 return NULL;
 
 if (qemuBuildSEVCommandLine(vm, cmd, def->sev) < 0)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f6683d11e0..2dbe6f6454 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5149,6 +5149,16 @@ qemuDomainDefValidateFeatures(const virDomainDef *def,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_VMCOREINFO:
+if (def->features[i] == VIR_TRISTATE_SWITCH_ON &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMCOREINFO)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+  _("vmcoreinfo is not available "
+"with this QEMU binary"));
+return -1;
+}
+break;
+
 case VIR_DOMAIN_FEATURE_ACPI:
 case VIR_DOMAIN_FEATURE_APIC:
 case VIR_DOMAIN_FEATURE_PAE:
@@ -5159,7 +5169,6 @@ qemuDomainDefValidateFeatures(const virDomainDef *def,
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
 case VIR_DOMAIN_FEATURE_CAPABILITIES:
 case VIR_DOMAIN_FEATURE_PMU:
-case VIR_DOMAIN_FEATURE_VMCOREINFO:
 case VIR_DOMAIN_FEATURE_MSRS:
 case VIR_DOMAIN_FEATURE_LAST:
 break;
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 64321bcb80..5ab00e5552 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1330,7 +1330,7 @@ mymain(void)
 DO_TEST("cpu-check-default-none2", NONE);
 DO_TEST("cpu-check-default-partial", NONE);
 DO_TEST("cpu-check-default-partial2", NONE);
-DO_TEST("vmcoreinfo", NONE);
+DO_TEST("vmcoreinfo", QEMU_CAPS_DEVICE_VMCOREINFO);
 
 DO_TEST("smartcard-host", QEMU_CAPS_CCID_EMULATED);
 DO_TEST("smartcard-host-certificates", QEMU_CAPS_CCID_EMULATED);
-- 
2.23.0


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

[libvirt] [PATCH v2 2/4] qemu: command: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c

2019-12-17 Thread Daniel Henrique Barboza

Move EGL Headless validation from qemuBuildGraphicsEGLHeadlessCommandLine()
to qemuDomainDeviceDefValidateGraphics(). This function is called by
qemuDomainDefValidate(), validating the graphics parameters in domain
define time.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_command.c | 10 +-
 src/qemu/qemu_domain.c  |  8 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3267b0d4b5..b3f069d5d4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7733,7 +7733,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr 
cfg,
 static int
 qemuBuildGraphicsEGLHeadlessCommandLine(virQEMUDriverConfigPtr cfg 
G_GNUC_UNUSED,
 virCommandPtr cmd,
-virQEMUCapsPtr qemuCaps,
 virDomainGraphicsDefPtr graphics)
 {
 g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER;
@@ -7741,13 +7740,6 @@ 
qemuBuildGraphicsEGLHeadlessCommandLine(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED
 virBufferAddLit(&opt, "egl-headless");
 
 if (graphics->data.egl_headless.rendernode) {
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS_RENDERNODE)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("This QEMU doesn't support OpenGL rendernode "
- "with egl-headless graphics type"));
-return -1;
-}
-
 virBufferAddLit(&opt, ",rendernode=");
 virQEMUBuildBufferEscapeComma(&opt,
   graphics->data.egl_headless.rendernode);
@@ -7792,7 +7784,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
 break;
 case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS:
 if (qemuBuildGraphicsEGLHeadlessCommandLine(cfg, cmd,
-qemuCaps, graphics) < 
0)
+graphics) < 0)
 return -1;
 
 break;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 83964db595..fe353e5bc1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7941,6 +7941,14 @@ qemuDomainDeviceDefValidateGraphics(const 
virDomainGraphicsDef *graphics,
 break;
 
 case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS:
+if (graphics->data.egl_headless.rendernode &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS_RENDERNODE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("This QEMU doesn't support OpenGL rendernode "
+ "with egl-headless graphics type"));
+return -1;
+}
+
 break;
 case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
 case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
-- 
2.23.0


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

[libvirt] [PATCH v2 0/4] move qemucaps validations from qemu_command to qemu_domain

2019-12-17 Thread Daniel Henrique Barboza

Most of the patches were pushed in the first version. These
are the patches that didn't make the cut and needed new
versions.

After these patches I'll refrain from doing more validation
moves to qemu_domain.c. I'll send a RFC discussing how we can
move all the validations to a new file first, then we can
resume this work in the definitive location instead. 


Changes from previous version 1 [1]:
- patch 1 (former patch 04): moved the validation to a new
qemuDomainDeviceDefValidateMemory() function which is called by
qemuDomainDeviceDefValidate().
- patch 2 (former patch 23): added the missing
"if (graphics->data.egl_headless.rendernode)" check.
- patch 3 (former patch 24): virReportEnumRangeError is now being
used in the "default" label error case.
- patch 4 (new): added the VMCOREINFO validation case that I
mentioned about in the cover letter of [1] as a difficult
case, but Cole mentioned that it was simpler than that and that I
was probably missing something. Cole was right.

[1] https://www.redhat.com/archives/libvir-list/2019-December/msg00570.html


Daniel Henrique Barboza (4):
  qemu: command: move NVDIMM validation to qemu_domain.c
  qemu: command: move qemuBuildGraphicsEGLHeadlessCommandLine validation
to qemu_domain.c
  qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c
  qemu: command: move validation of vmcoreinfo to qemu_domain.c

 src/qemu/qemu_command.c | 51 ++
 src/qemu/qemu_domain.c  | 70 +++--
 tests/qemuxml2xmltest.c | 30 +-
 3 files changed, 87 insertions(+), 64 deletions(-)

-- 
2.23.0


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

Re: [libvirt] [PATCH v6 0/4] PCI hostdev partial assignment support

2019-12-17 Thread Alex Williamson

On Tue, 17 Dec 2019 17:35:01 -0300
Daniel Henrique Barboza  wrote:

> changes from previous version 5 [1]:
> - changes in the commit message of patch 1 and the
> documentation included in patches 3 and 4, all of them
> suggested/hinted by Alex Williamson.


Seems conceptually sound and believe the descriptions are accurate now,
I'll leave it to libvirt folks to determine that it does what it says
in a reasonable way ;)  Thanks,

Alex

 
> Daniel Henrique Barboza (4):
>   Introducing new address type='unassigned' for PCI hostdevs
>   qemu: handle unassigned PCI hostdevs in command line
>   formatdomain.html.in: document 
>   news.xml: add address type='unassigned' entry
> 
> 
> [1] https://www.redhat.com/archives/libvir-list/2019-December/msg01097.html
> 
>  docs/formatdomain.html.in | 10 
>  docs/news.xml | 14 +
>  docs/schemas/domaincommon.rng |  5 ++
>  src/conf/device_conf.c|  2 +
>  src/conf/device_conf.h|  1 +
>  src/conf/domain_conf.c|  7 ++-
>  src/qemu/qemu_command.c   |  5 ++
>  src/qemu/qemu_domain.c|  1 +
>  src/qemu/qemu_domain_address.c|  5 ++
>  .../hostdev-pci-address-unassigned.args   | 31 ++
>  .../hostdev-pci-address-unassigned.xml| 42 ++
>  tests/qemuxml2argvtest.c  |  4 ++
>  .../hostdev-pci-address-unassigned.xml| 58 +++
>  tests/qemuxml2xmltest.c   |  1 +
>  14 files changed, 185 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
>  create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
>  create mode 100644 
> tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml
> 

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

[libvirt] [PATCH v3 4/4] util: Rewrite virGetUserRuntimeDirectory() using g_get_user_runtime_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 39 ++-
 1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 2e8b4e95b7..52b12126d7 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -606,6 +606,16 @@ char *virGetUserCacheDirectory(void)
 }
 
 
+char *virGetUserRuntimeDirectory(void)
+{
+#ifdef WIN32
+return g_strdup(g_get_user_runtime_dir());
+#else
+return g_strdup_printf("%s/libvirt", g_get_user_runtime_dir());
+#endif
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -756,20 +766,6 @@ char *virGetUserShell(uid_t uid)
 }
 
 
-char *virGetUserRuntimeDirectory(void)
-{
-const char *path = getenv("XDG_RUNTIME_DIR");
-
-if (!path || !path[0]) {
-return virGetUserCacheDirectory();
-} else {
-char *ret;
-
-ret = g_strdup_printf("%s/libvirt", path);
-return ret;
-}
-}
-
 char *virGetUserName(uid_t uid)
 {
 char *ret;
@@ -1179,12 +1175,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserRuntimeDirectory(void)
-{
-return virGetUserCacheDirectory();
-}
-
 # else /* !HAVE_GETPWUID_R && !WIN32 */
 char *
 virGetUserDirectoryByUID(uid_t uid G_GNUC_UNUSED)
@@ -1203,15 +1193,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 
 return NULL;
 }
-
-char *
-virGetUserRuntimeDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserRuntimeDirectory is not available"));
-
-return NULL;
-}
 # endif /* ! HAVE_GETPWUID_R && ! WIN32 */
 
 char *
-- 
2.23.0

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

[libvirt] [PATCH v3 0/4] Rewrite virGetUserDirectory() functions using g_get__dir()

2019-12-17 Thread Fabiano Fidêncio

By rewriting virGetUser*Directory() functions using g_get_*_dir()
functions allows us to drop all the different implementations we
keep, as GLib already takes care of those for us.

Changes since v2:
https://www.redhat.com/archives/libvir-list/2019-December/msg01064.html
- Addressed comments made by Cole about:
  - virGetUserDirectory() should return $HOME;
  - virGetUser*Directory() don't append "libvirt" to the returned path
on Windows;

Changes since v1:
https://www.redhat.com/archives/libvir-list/2019-December/msg01055.html
- Don't check for the return of g_get_*_dir(), as it cannot be NULL;

Fabiano Fidêncio (4):
  util: Rewrite virGetUserDirectory() using g_get_home_dir()
  util: Rewrite virGetUserConfigDirectory() using
g_get_user_config_dir()
  util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir()
  util: Rewrite virGetUserRuntimeDirectory() using
g_get_user_runtime_dir()

 src/util/virutil.c | 137 ++---
 1 file changed, 31 insertions(+), 106 deletions(-)

-- 
2.23.0

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

[libvirt] [PATCH v3 3/4] util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 57 --
 1 file changed, 10 insertions(+), 47 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 1bcdde9ad6..2e8b4e95b7 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -596,6 +596,16 @@ char *virGetUserConfigDirectory(void)
 }
 
 
+char *virGetUserCacheDirectory(void)
+{
+#ifdef WIN32
+return g_strdup(g_get_user_cache_dir());
+#else
+return g_strdup_printf("%s/libvirt", g_get_user_cache_dir());
+#endif
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -746,29 +756,6 @@ char *virGetUserShell(uid_t uid)
 }
 
 
-static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)
-{
-const char *path = getenv(xdgenvname);
-char *ret = NULL;
-char *home = NULL;
-
-if (path && path[0]) {
-ret = g_strdup_printf("%s/libvirt", path);
-} else {
-home = virGetUserDirectory();
-if (home)
-ret = g_strdup_printf("%s/%s/libvirt", home, xdgdefdir);
-}
-
-VIR_FREE(home);
-return ret;
-}
-
-char *virGetUserCacheDirectory(void)
-{
-return virGetXDGDirectory("XDG_CACHE_HOME", ".cache");
-}
-
 char *virGetUserRuntimeDirectory(void)
 {
 const char *path = getenv("XDG_RUNTIME_DIR");
@@ -1192,21 +1179,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserCacheDirectory(void)
-{
-char *ret;
-if (virGetWin32SpecialFolder(CSIDL_INTERNET_CACHE, &ret) < 0)
-return NULL;
-
-if (!ret) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to determine config directory"));
-return NULL;
-}
-return ret;
-}
-
 char *
 virGetUserRuntimeDirectory(void)
 {
@@ -1232,15 +1204,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserCacheDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserCacheDirectory is not available"));
-
-return NULL;
-}
-
 char *
 virGetUserRuntimeDirectory(void)
 {
-- 
2.23.0

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

[libvirt] [PATCH v3 1/4] util: Rewrite virGetUserDirectory() using g_get_home_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index ed1f696e37..7008c3119c 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -582,7 +582,7 @@ virGetHostnameQuiet(void)
 char *
 virGetUserDirectory(void)
 {
-return virGetUserDirectoryByUID(geteuid());
+return g_strdup(g_get_home_dir());
 }
 
 
-- 
2.23.0

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

[libvirt] [PATCH v3 2/4] util: Rewrite virGetUserConfigDirectory() using g_get_user_config_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 39 ++-
 1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 7008c3119c..1bcdde9ad6 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -586,6 +586,16 @@ virGetUserDirectory(void)
 }
 
 
+char *virGetUserConfigDirectory(void)
+{
+#ifdef WIN32
+return g_strdup(g_get_user_config_dir());
+#else
+return g_strdup_printf("%s/libvirt", g_get_user_config_dir());
+#endif
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -754,11 +764,6 @@ static char *virGetXDGDirectory(const char *xdgenvname, 
const char *xdgdefdir)
 return ret;
 }
 
-char *virGetUserConfigDirectory(void)
-{
-return virGetXDGDirectory("XDG_CONFIG_HOME", ".config");
-}
-
 char *virGetUserCacheDirectory(void)
 {
 return virGetXDGDirectory("XDG_CACHE_HOME", ".cache");
@@ -1187,21 +1192,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserConfigDirectory(void)
-{
-char *ret;
-if (virGetWin32SpecialFolder(CSIDL_LOCAL_APPDATA, &ret) < 0)
-return NULL;
-
-if (!ret) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to determine config directory"));
-return NULL;
-}
-return ret;
-}
-
 char *
 virGetUserCacheDirectory(void)
 {
@@ -1242,15 +1232,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserConfigDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserConfigDirectory is not available"));
-
-return NULL;
-}
-
 char *
 virGetUserCacheDirectory(void)
 {
-- 
2.23.0

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

[libvirt] [PATCH v6 3/4] formatdomain.html.in: document

2019-12-17 Thread Daniel Henrique Barboza

Signed-off-by: Daniel Henrique Barboza 
---
 docs/formatdomain.html.in | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e06cf2061b..dd04a05f09 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4203,6 +4203,16 @@
 attributes: iobase and irq.
 Since 1.2.1
   
+  unassigned
+  For PCI hostdevs, 
+allows the admin to include a PCI hostdev in the domain XML definition,
+without making it available for the guest. This allows for 
configurations
+in which Libvirt manages the device as a regular PCI hostdev,
+regardless of whether the guest will have access to it.
+ is an invalid address
+type for all other device types.
+Since 6.0.0
+  
 
 
 Virtio-related options
-- 
2.23.0


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

[libvirt] [PATCH v6 0/4] PCI hostdev partial assignment support

2019-12-17 Thread Daniel Henrique Barboza

changes from previous version 5 [1]:
- changes in the commit message of patch 1 and the
documentation included in patches 3 and 4, all of them
suggested/hinted by Alex Williamson.


Daniel Henrique Barboza (4):
  Introducing new address type='unassigned' for PCI hostdevs
  qemu: handle unassigned PCI hostdevs in command line
  formatdomain.html.in: document 
  news.xml: add address type='unassigned' entry


[1] https://www.redhat.com/archives/libvir-list/2019-December/msg01097.html

 docs/formatdomain.html.in | 10 
 docs/news.xml | 14 +
 docs/schemas/domaincommon.rng |  5 ++
 src/conf/device_conf.c|  2 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c|  7 ++-
 src/qemu/qemu_command.c   |  5 ++
 src/qemu/qemu_domain.c|  1 +
 src/qemu/qemu_domain_address.c|  5 ++
 .../hostdev-pci-address-unassigned.args   | 31 ++
 .../hostdev-pci-address-unassigned.xml| 42 ++
 tests/qemuxml2argvtest.c  |  4 ++
 .../hostdev-pci-address-unassigned.xml| 58 +++
 tests/qemuxml2xmltest.c   |  1 +
 14 files changed, 185 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml

-- 
2.23.0


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

[libvirt] [PATCH v6 1/4] Introducing new address type='unassigned' for PCI hostdevs

2019-12-17 Thread Daniel Henrique Barboza

This patch introduces a new PCI hostdev address type called
'unassigned'. This new type gives users the option to add
PCI hostdevs to the domain XML in an 'unassigned' state, meaning
that the device exists in the domain, is managed by Libvirt
like any regular PCI hostdev, but the guest does not have
access to it.

This adds extra options for managing PCI device binding
inside Libvirt, for example, making all the managed PCI hostdevs
declared in the domain XML to be detached from the host and bind
to the chosen driver and, at the same time, allowing just a
subset of these devices to be usable by the guest.

Next patch will use this new address type in the QEMU driver to
avoid adding unassigned devices to the QEMU launch command line.

Signed-off-by: Daniel Henrique Barboza 
---
 docs/schemas/domaincommon.rng |  5 ++
 src/conf/device_conf.c|  2 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c|  7 ++-
 src/qemu/qemu_command.c   |  1 +
 src/qemu/qemu_domain.c|  1 +
 src/qemu/qemu_domain_address.c|  5 ++
 .../hostdev-pci-address-unassigned.xml| 42 ++
 .../hostdev-pci-address-unassigned.xml| 58 +++
 tests/qemuxml2xmltest.c   |  1 +
 10 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e964773f5e..5f1d4a34a4 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5502,6 +5502,11 @@
   
   
 
+
+  
+unassigned
+  
+
   
 
   
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 4c57f0995f..4dbd5c1ac9 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress,
   "virtio-mmio",
   "isa",
   "dimm",
+  "unassigned",
 );
 
 static int
@@ -120,6 +121,7 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo 
*a,
 /* address types below don't have any specific data */
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index d98fae775c..e091d7cfe2 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -45,6 +45,7 @@ typedef enum {
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO,
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA,
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM,
+VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED,
 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
 } virDomainDeviceAddressType;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e46f423b19..afa072e17d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6352,9 +6352,11 @@ virDomainHostdevDefValidate(const virDomainHostdevDef 
*hostdev)
 switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
 if (hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+hostdev->info->type != 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED &&
 hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("PCI host devices must use 'pci' address 
type"));
+   _("PCI host devices must use 'pci' or "
+ "'unassigned' address type"));
 return -1;
 }
 break;
@@ -7371,6 +7373,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
 break;
 }
@@ -7571,6 +7574,7 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
 break;
 }
@@ -21951,6 +21955,7 @@ 
virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
 break;
 }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 836057a4ff..

[libvirt] [PATCH v6 4/4] news.xml: add address type='unassigned' entry

2019-12-17 Thread Daniel Henrique Barboza

Signed-off-by: Daniel Henrique Barboza 
---
 docs/news.xml | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2a25b6ca49..055353b9a5 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -54,6 +54,20 @@
   written in the RST as an alternative to HTML.
 
   
+  
+
+  new PCI hostdev address type: unassigned
+
+
+  A new PCI hostdev address type 'unassigned' is introduced. An
+  unassigned PCI hostdev behaves like any regular PCI hostdev
+  inside Libvirt, but it is not usable by the guest. This gives
+  the user a new option to manage the binding of PCI devices
+  via Libvirt, declaring PCI hostdevs in the domain XML
+  but allowing just a subset of them to be assigned to the
+  guest.
+
+  
 
 
   
-- 
2.23.0


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

[libvirt] [PATCH v6 2/4] qemu: handle unassigned PCI hostdevs in command line

2019-12-17 Thread Daniel Henrique Barboza

Previous patch made it possible for the QEMU driver to check if
a given PCI hostdev is unassigned, by checking if dev->info->type is
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED, meaning that this device
shouldn't be part of the actual guest launch.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_command.c   |  4 +++
 .../hostdev-pci-address-unassigned.args   | 31 +++
 tests/qemuxml2argvtest.c  |  4 +++
 3 files changed, 39 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 864967f375..ed8b878566 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5336,6 +5336,10 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
 *bootHostdevNet = 0;
 }
 
+   /* Ignore unassigned devices  */
+   if (hostdev->info->type == 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED)
+   continue;
+
 if (qemuCommandAddExtDevice(cmd, hostdev->info) < 0)
 return -1;
 
diff --git a/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args 
b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
new file mode 100644
index 00..42fae17444
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-delete \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-delete/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-delete/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-delete/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name delete \
+-S \
+-machine pc,accel=kvm,usb=off,dump-guest-core=off \
+-m 256 \
+-realtime mlock=off \
+-smp 4,sockets=4,cores=1,threads=1 \
+-uuid 583a8e8e-f0ce-4f53-89ab-092862148b25 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-delete/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=0005:90:01.2,id=hostdev2,bus=pci.0,addr=0x4 \
+-device vfio-pci,host=0005:90:01.3,id=hostdev3,bus=pci.0,addr=0x5 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 118a460633..bfbed5c31d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1332,6 +1332,10 @@ mymain(void)
 QEMU_CAPS_KVM,
 QEMU_CAPS_DEVICE_VFIO_PCI);
 
+DO_TEST("hostdev-pci-address-unassigned",
+QEMU_CAPS_KVM,
+QEMU_CAPS_DEVICE_VFIO_PCI);
+
 DO_TEST("serial-file-log",
 QEMU_CAPS_CHARDEV_FILE_APPEND,
 QEMU_CAPS_DEVICE_ISA_SERIAL,
-- 
2.23.0


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

[libvirt] [PATCH 3/3] virsh: Adjust logic checks in virshUpdateDiskXML

2019-12-17 Thread John Ferlan

Make it clearer that what we're trying to do is find @source and
@target_node so that the unattentive or code analysis utility
doesn't believe 'source' and 'target' could be found in the same
node element.

Signed-off-by: John Ferlan 
---
 tools/virsh-domain.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 56137bdd74..9d4cdd26dd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12476,10 +12476,9 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
 if (tmp->type != XML_ELEMENT_NODE)
 continue;
 
-if (virXMLNodeNameEqual(tmp, "source"))
+if (!source && virXMLNodeNameEqual(tmp, "source"))
 source = tmp;
-
-if (virXMLNodeNameEqual(tmp, "target"))
+else if (!target_node && virXMLNodeNameEqual(tmp, "target"))
 target_node = tmp;
 
 /*
-- 
2.23.0

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

[libvirt] [PATCH 2/3] vbox: Reset @ret after xmlFreeNode

2019-12-17 Thread John Ferlan

In the error path, if we xmlFreeNode @ret, then the return ret;
a few lines later returns something that's already been free'd
and could be reused, so let's reinit it.

Found by Coverity

Signed-off-by: John Ferlan 
---
 src/vbox/vbox_snapshot_conf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
index 5a0abd6d0e..db6c389a64 100644
--- a/src/vbox/vbox_snapshot_conf.c
+++ b/src/vbox/vbox_snapshot_conf.c
@@ -352,6 +352,7 @@ 
virVBoxSnapshotConfCreateHardDiskNode(virVBoxSnapshotConfHardDiskPtr hardDisk)
 if (result < 0) {
 xmlUnlinkNode(ret);
 xmlFreeNode(ret);
+ret = NULL;
 }
 VIR_FREE(uuid);
 return ret;
-- 
2.23.0

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

[libvirt] [PATCH 1/3] conf: Fix ATTRIBUTE_NONNULL usages

2019-12-17 Thread John Ferlan

Recent changes removed the virCapsPtr, but didn't adjust/remove the
corresponding ATTRIBUTE_NONNULL resulting in a build failure to build
in my Coverity environment.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.h | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 11fafe46b3..e012975fca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3089,27 +3089,24 @@ unsigned int virDomainDefFormatConvertXMLFlags(unsigned 
int flags);
 char *virDomainDefFormat(virDomainDefPtr def,
  virDomainXMLOptionPtr xmlopt,
  unsigned int flags)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(3);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 char *virDomainObjFormat(virDomainObjPtr obj,
  virDomainXMLOptionPtr xmlopt,
  unsigned int flags)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(3);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 int virDomainDefFormatInternal(virDomainDefPtr def,
virDomainXMLOptionPtr xmlopt,
virBufferPtr buf,
unsigned int flags)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+ATTRIBUTE_NONNULL(3);
 int virDomainDefFormatInternalSetRootName(virDomainDefPtr def,
   virDomainXMLOptionPtr xmlopt,
   virBufferPtr buf,
   const char *rootname,
   unsigned int flags)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
-ATTRIBUTE_NONNULL(5);
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
 
 int virDomainDiskSourceFormat(virBufferPtr buf,
   virStorageSourcePtr src,
@@ -3286,14 +3283,14 @@ int virDomainDefSave(virDomainDefPtr def,
  const char *configDir)
 G_GNUC_WARN_UNUSED_RESULT
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+ATTRIBUTE_NONNULL(3);
 
 int virDomainObjSave(virDomainObjPtr obj,
  virDomainXMLOptionPtr xmlopt,
  const char *statusDir)
 G_GNUC_WARN_UNUSED_RESULT
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+ATTRIBUTE_NONNULL(3);
 
 typedef void (*virDomainLoadConfigNotify)(virDomainObjPtr dom,
   int newDomain,
-- 
2.23.0

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

[libvirt] [PATCH 0/3] Some coverity adjustments

2019-12-17 Thread John Ferlan

I upgraded to f31 and it resulted in an essentially hosed Coverity
build/analysis environment with the following message during cov-emit
processing (a preprocessing of sorts):

"/usr/include/glib-2.0/glib/gspawn.h", line 76: error #67: expected a "}"
G_SPAWN_ERROR_2BIG 
GLIB_DEPRECATED_ENUMERATOR_IN_2_32_FOR(G_SPAWN_ERROR_TOO_BIG) = 
G_SPAWN_ERROR_TOO_BIG,
   ^

So instead, I'm using a guest to run Coverity "when I remember/can".

I also found that my f31 environment doesn't like building w/ docs as
I get the following messages while running the convert command:

...
usr/bin/mv: cannot stat '/tmp/magick-1191987h12h27ex0lZD.svg': No such file or 
directory
  GEN  kbase.html.tmp
convert: delegate failed `'uniconvertor' '%i' '%o.svg'; /usr/bin/mv '%o.svg' 
'%o'' @ error/delegate.c/InvokeDelegate/1958.
convert: unable to open file `/tmp/magick-1191987OqYJwrq8isaG': No such file or 
directory @ error/constitute.c/ReadImage/605.
convert: no images defined `migration-managed-p2p.png' @ 
error/convert.c/ConvertImageCommand/3235.


I haven't followed along as closely as I used to, but my vpath env
uses obj as a subdirectory of my main git tree/target. Whether the
new build env has anything to do with it or it's just f31, I haven't
been able to determine.


Beyond these 3 patches here - there is one other adjustment that is
necessary to build libvirt under Coverity and that's removing the
ATTRIBUTE_NONNULL(2) from the virDomainDefFormat definition in
src/conf/domain_conf.h.  This was added in commit 92d412149 which
also included two calls to virDomainDefFormat with NULL as the 2nd
argument (hyperv_driver.c and security_apparmor.c); however, the
commit message notes preparation for future work, so I'll keep a
hack for that local for now at least.

The virsh change below is innocuous yes, but it showed up in a
coverity analysis because it wasn't sure if the resulting variables
could point to the same address and if they did, then there was a
possible use after free because the @source is free'd even though
the @target_node is later referenced. The patch here avoids that
and provides a slight adjustment to not search for either node by
name if it was already found. Whether there's a weird latent issue
because  can be repeated while  cannot be is something
I suppose a reviewer can warn me about ;-).

John Ferlan (3):
  conf: Fix ATTRIBUTE_NONNULL usages
  vbox: Reset @ret after xmlFreeNode
  virsh: Adjust logic checks in virshUpdateDiskXML

 src/conf/domain_conf.h| 15 ++-
 src/vbox/vbox_snapshot_conf.c |  1 +
 tools/virsh-domain.c  |  5 ++---
 3 files changed, 9 insertions(+), 12 deletions(-)

-- 
2.23.0

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

Re: [libvirt] [PATCH v2 2/4] util: Rewrite virGetUserConfigDirectory() using g_get_user_config_dir()

2019-12-17 Thread Cole Robinson

On 12/17/19 11:41 AM, Fabiano Fidêncio wrote:
> Signed-off-by: Fabiano Fidêncio 
> ---
>  src/util/virutil.c | 35 ++-
>  1 file changed, 6 insertions(+), 29 deletions(-)
> 
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 8c255abd7f..63680974b8 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -586,6 +586,12 @@ virGetUserDirectory(void)
>  }
>  
>  
> +char *virGetUserConfigDirectory(void)
> +{
> +return g_strdup_printf("%s/libvirt", g_get_user_config_dir());
> +}
> +
> +
>  #ifdef HAVE_GETPWUID_R
>  /* Look up fields from the user database for the given user.  On
>   * error, set errno, report the error if not instructed otherwise via @quiet,
> @@ -754,11 +760,6 @@ static char *virGetXDGDirectory(const char *xdgenvname, 
> const char *xdgdefdir)
>  return ret;
>  }
>  
> -char *virGetUserConfigDirectory(void)
> -{
> -return virGetXDGDirectory("XDG_CONFIG_HOME", ".config");
> -}
> -
>  char *virGetUserCacheDirectory(void)
>  {
>  return virGetXDGDirectory("XDG_CACHE_HOME", ".cache");
> @@ -1187,21 +1188,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
>  return NULL;
>  }
>  
> -char *
> -virGetUserConfigDirectory(void)
> -{
> -char *ret;
> -if (virGetWin32SpecialFolder(CSIDL_LOCAL_APPDATA, &ret) < 0)
> -return NULL;
> -
> -if (!ret) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("Unable to determine config directory"));
> -return NULL;
> -}
> -return ret;
> -}
> -

Unfortunately looks like the windows implementations do _not_ add
'/libvirt' to the returned directory. I'm doubtful anyone is depending
on these paths being stable, but for safety we should differentiate
this. I think it's fine to do:

#ifdef WIN32
return g_strdup(g_get_blah());
#else
return g_strdup_print(...);
#endif

Rather than have two different functions

Also, a couple ideas for a follow up series: all the error handling from
the GetUser*Directory functions can be removed after this. Also, the it
looks like the windows impl of virGetUserDirectoryByUID can be entirely
replaced with g_get_user_home(), which allows dropping some more windows
helpers (the code was apparently adapted from glib anyways)

- Cole

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

Re: [libvirt] [PATCH v5 4/4] news.xml: add address type='unassigned' entry

2019-12-17 Thread Daniel Henrique Barboza





On 12/17/19 4:44 PM, Alex Williamson wrote:

On Tue, 17 Dec 2019 16:06:47 -0300
Daniel Henrique Barboza  wrote:


Signed-off-by: Daniel Henrique Barboza 
---
  docs/news.xml | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2a25b6ca49..febda970f6 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -54,6 +54,20 @@
written in the RST as an alternative to HTML.
  

+  
+
+  new PCI hostdev address type: unassigned
+
+
+  A new PCI hostdev address type 'unassigned' is introduced,
+  giving users the option to choose which PCI hostdevs
+  within the same IOMMU group will not be assigned to the
+  guest. PCI hostdevs that shouldn't be used by the guest
+  can be classified as address type='unassigned'.
+  Libvirt will still be able to manage the device as a
+  regular PCI hostdev.
+


This is rather convoluted.  Users have always had the choice of which
devices NOT to assign.  I would present this as a mechanism for libvirt
to manage the driver binding of devices, as done for managed PCI hostdev
devices, without actually attaching the devices to the VM, thereby
facilitating device assignment when only a subset of the devices within
an IOMMU group are intended to be used by the guest.



Thanks. I'll resend the series with a better tone in the docs and the
commit message of patch 1.




BTW, this should also work with the hostdev  option if
the user wanted to bind the device to pci-stub rather than vfio-pci to
provide even further isolation from the user being able to access the
device.  Thanks,


After dropping the heavy validation logic I was doing in the previous versions,
it should work with driver='kvm' as well. What the code is doing now is
removing any device marked as unassigned from QEMU launch command. If QEMU is
fine with the pci-stub setup the user is setting up, this code will not stand
in the way.


Thanks,


DHB



Alex


+  
  
  





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

Re: [libvirt] [PATCH v2 1/4] util: Rewrite virGetUserDirectory() using g_get_home_dir()

2019-12-17 Thread Cole Robinson

On 12/17/19 11:41 AM, Fabiano Fidêncio wrote:
> Signed-off-by: Fabiano Fidêncio 
> ---
>  src/util/virutil.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index ed1f696e37..8c255abd7f 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -582,7 +582,7 @@ virGetHostnameQuiet(void)
>  char *
>  virGetUserDirectory(void)
>  {
> -return virGetUserDirectoryByUID(geteuid());
> +return g_strdup_printf("%s/libvirt", g_get_home_dir());
>  }

This is supposed to be returning $HOME, not $HOME/libvirt. IMO leave
this one as it is, since it's tied into the larger getent handling

- Cole

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

Re: [libvirt] [PATCH v5 4/4] news.xml: add address type='unassigned' entry

2019-12-17 Thread Alex Williamson

On Tue, 17 Dec 2019 16:06:47 -0300
Daniel Henrique Barboza  wrote:

> Signed-off-by: Daniel Henrique Barboza 
> ---
>  docs/news.xml | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/docs/news.xml b/docs/news.xml
> index 2a25b6ca49..febda970f6 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -54,6 +54,20 @@
>written in the RST as an alternative to HTML.
>  
>
> +  
> +
> +  new PCI hostdev address type: unassigned
> +
> +
> +  A new PCI hostdev address type 'unassigned' is introduced,
> +  giving users the option to choose which PCI hostdevs
> +  within the same IOMMU group will not be assigned to the
> +  guest. PCI hostdevs that shouldn't be used by the guest
> +  can be classified as address type='unassigned'.
> +  Libvirt will still be able to manage the device as a
> +  regular PCI hostdev.
> +

This is rather convoluted.  Users have always had the choice of which
devices NOT to assign.  I would present this as a mechanism for libvirt
to manage the driver binding of devices, as done for managed PCI hostdev
devices, without actually attaching the devices to the VM, thereby
facilitating device assignment when only a subset of the devices within
an IOMMU group are intended to be used by the guest.

BTW, this should also work with the hostdev  option if
the user wanted to bind the device to pci-stub rather than vfio-pci to
provide even further isolation from the user being able to access the
device.  Thanks,

Alex

> +  
>  
>  
>

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

Re: [libvirt] [PATCH] qemu: homogenize MAC address in live & config when hotplugging a netdev

2019-12-17 Thread Laine Stump


On 12/17/19 1:45 PM, Michal Prívozník wrote:

On 12/17/19 6:04 PM, Laine Stump wrote:


...


+static void
+qemuDomainAttachDeviceLiveAndConfigHomogenize(const virDomainDeviceDef 
*devConf,
+  virDomainDeviceDefPtr devLive)
+{
+/*
+ * fixup anything that needs to be identical in the live and


s/fixup/Fixup/


+ * config versions of DeviceDef, but might not be. Do this by
+ * changing the contents of devLive.


We need to warn everybody that only "small" changes are okay. I mean, we
probably don't have to explicitly warn about not changing PCI address,
but something among these lines should do:


Yeah, I thought of adding a warning, and actually was going to 
specifically mention that PCI/etc addresses shouldn't be changed (since 
that was something people have already talked about adjusting), but then 
forgot. I'll add in something like you suggest before pushing.


Thanks!


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

Re: [libvirt] [PATCH 4/7] libvirt: support an "embed" URI path selector for opening drivers

2019-12-17 Thread Cole Robinson

On 12/17/19 2:28 PM, Michal Prívozník wrote:
> 
> But this looks weird, isn't virt-install registerin an event loop? How
> else does it get events?

virt-install doesn't register an event loop, it polls for the single VM
state change we care about. But if registering an event loop is
necessary for qemu:///embed usage then it's worth adding the libvirtglib
init to virt-install, if only for testing purposes.

- Cole

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

Re: [libvirt] [PATCH v5 3/4] formatdomain.html.in: document

2019-12-17 Thread Alex Williamson

On Tue, 17 Dec 2019 16:06:46 -0300
Daniel Henrique Barboza  wrote:

> Signed-off-by: Daniel Henrique Barboza 
> ---
>  docs/formatdomain.html.in | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index e06cf2061b..7a5ebdd67e 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4203,6 +4203,19 @@
>  attributes: iobase and irq.
>  Since 1.2.1
>
> +  unassigned
> +  For PCI hostdevs, 
> +allows the admin to include a PCI hostdev in the domain XML 
> definition,
> +without making it available for the guest. This allows for 
> configurations
> +in which Libvirt manages the device as a regular PCI hostdev,
> +regardless of whether the guest will have access to it. This is
> +an alternative to scenarios in which the admin might be compelled to 
> use
> +an ACS patch to remove the device from the guest while Libvirt
> +retains control of the PCI device.

The ACS patch is really orthogonal to the goal here, so I don't think
it should be included in the discussion.  A user can just as easily
pre-bind other devices to vfio-pci to make the configuration viable
rather than patch their kernel to change the viability constraints,
which this series doesn't accomplish either.  Thanks,

Alex

> + is an invalid address
> +type for all other device types.
> +Since 6.0.0
> +  
>  
>  
>  Virtio-related options

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

Re: [libvirt] [PATCH 4/4] kbase: Add document outlining backing chain XML config and troubleshooting

2019-12-17 Thread Peter Krempa

On Tue, Dec 17, 2019 at 13:17:22 -0600, Eric Blake wrote:
> On 12/17/19 12:35 PM, Peter Krempa wrote:
> > Signed-off-by: Peter Krempa 
> > ---

[...]

> > +If the ``backing file format:`` field is missing above the format was not
> > +specified properly. The image can be fixed by the following command:
> > +
> > +::
> > +
> > + qemu-img rebase -f $IMAGE_FORMAT -F $BACKING_IMAGE_FORMAT -b 
> > $BACKING_IMAGE_PATH $IMAGE_PATH
> 
> Adding -u can make this operation faster (blindly update the image metadata,
> rather than actually crawling the entire image to perform a data operation
> in the process).  But I'm not sure whether we want to document that here.

I thought qemu would skip the data verification if the backing image is
not being changed. I'm not sure about the -u here though.

> > +It is important to fill out ``$BACKING_IMAGE_FORMAT`` and ``$IMAGE_FORMAT``
> > +properly. ``$BACKING_IMAGE_PATH`` should be specified as a full absolute 
> > path.
> > +If relative referencing of the backing image is desired, the path must be
> > +relative to the location of image described by ``$IMAGE_PATH``.
> > +
> > +Missing images reported after after moving disk images into a different 
> > path
> > +
> > +
> > +The path to the backing image which is recorded in the image metadata often
> > +contains a full path to the backing image. This is the default 
> > libvirt-created
> > +image configuration. When such images are moved to a different location the
> > +top image will no longer point to the correct image.
> > +
> > +To fix such issue you can either fully specify the image chain in the 
> > domain XML
> > +as pointed out above or the following ``qemu-img`` command can be used:
> > +
> > +::
> > +
> > + qemu-img rebase -u -f $IMAGE_FORMAT -F $BACKING_IMAGE_FORMAT -b 
> > $BACKING_IMAGE_PATH $IMAGE_PATH
> > +
> 
> Odd to mention -u here but not above.  Should we be consistent between the
> two examples?

You must use it here since qemu-img will fail to locate the old backing
file as the example is fixing a image chain using absolute paths moved
to a different location.

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

Re: [libvirt] [PATCH 4/7] libvirt: support an "embed" URI path selector for opening drivers

2019-12-17 Thread Michal Prívozník

On 12/17/19 6:41 PM, Cole Robinson wrote:
> On 12/2/19 10:03 AM, Daniel P. Berrangé wrote:
>> The driver URI scheme:
>>
>>   "$drivername:///embed?root=/some/path"
>>
>> enables a new way to use the drivers by embedding them directly in the
>> calling process. To use this the process must have a thread running the
>> libvirt event loop. This URI will then cause libvirt to dynamically load
>> the driver module and call its global initialization function. This
>> syntax is applicable to any driver, but only those will have been
>> modified to support a custom root directory and embed URI path will
>> successfully open.
>>
>> The application can now make normal libvirt API calls which are all
>> serviced in-process with no RPC layer involved.
>>
>> It is required to specify an explicit root directory, and locks will be
>> acquired on this directory to avoid conflicting with another app that
>> might accidentally pick the same directory.
>>
>> Use of '/' is not explicitly forbidden, but note that the file layout
>> used underneath the embedded driver root does not match the file
>> layout used by system/session mode drivers. So this cannot be used as
>> a backdoor to interact with, or fake, the system/session mode drivers.
>>
>> Libvirt will create arbitrary files underneath this root directory. The
>> root directory can be kept untouched across connection open attempts if
>> the application needs persistence. The application is responsible for
>> purging everything underneath this root directory when finally no longer
>> required.
>>
>> Even when a virt driver is used in embedded mode, it is still possible
>> for it to in turn use functionality that calls out to other secondary
>> drivers in libvirtd. For example an embedded instance of QEMU can open
>> the network, secret or storage drivers in the system libvirtd.
>>
>> That said, the application would typically want to at least open an
>> embedded secret driver ("secret:///embed?root=/some/path"). Note that
>> multiple different embedded drivers can use the same root prefix and
>> co-operate just as they would inside a normal libvirtd daemon.
>>
>> A key thing to note is that for this to work, the application that links
>> to libvirt *MUST* be built with -Wl,--export-dynamic to ensure that
>> symbols from libvirt.so are exported & thus available to the dynamically
>> loaded driver module. If libvirt.so itself was dynamically loaded then
>> RTLD_GLOBAL must be passed to dlopen().
>>
>> Signed-off-by: Daniel P. Berrangé 
>> ---
>>  src/driver-state.h |  1 +
>>  src/driver.h   |  2 ++
>>  src/libvirt.c  | 72 --
>>  3 files changed, 73 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/driver-state.h b/src/driver-state.h
>> index 1e2f6ed247..6b3f501e05 100644
>> --- a/src/driver-state.h
>> +++ b/src/driver-state.h
>> @@ -50,6 +50,7 @@ typedef virStateDriver *virStateDriverPtr;
>>  
>>  struct _virStateDriver {
>>  const char *name;
>> +bool initialized;
>>  virDrvStateInitialize stateInitialize;
>>  virDrvStateCleanup stateCleanup;
>>  virDrvStateReload stateReload;
>> diff --git a/src/driver.h b/src/driver.h
>> index ca82ac974b..6278aa05b3 100644
>> --- a/src/driver.h
>> +++ b/src/driver.h
>> @@ -82,6 +82,8 @@ struct _virConnectDriver {
>>  bool localOnly;
>>  /* Whether driver needs a server in the URI */
>>  bool remoteOnly;
>> +/* Whether driver can be used in embedded mode */
>> +bool embeddable;
>>  /*
>>   * NULL terminated list of supported URI schemes.
>>   *  - Single element { NULL } list indicates no supported schemes
>> diff --git a/src/libvirt.c b/src/libvirt.c
>> index bd2952d036..17b6506faa 100644
>> --- a/src/libvirt.c
>> +++ b/src/libvirt.c
>> @@ -52,6 +52,7 @@
>>  # include "rpc/virnettlscontext.h"
>>  #endif
>>  #include "vircommand.h"
>> +#include "virevent.h"
>>  #include "virfile.h"
>>  #include "virrandom.h"
>>  #include "viruri.h"
>> @@ -84,6 +85,7 @@
>>  #ifdef WITH_BHYVE
>>  # include "bhyve/bhyve_driver.h"
>>  #endif
>> +#include "access/viraccessmanager.h"
>>  
>>  #define VIR_FROM_THIS VIR_FROM_NONE
>>  
>> @@ -676,10 +678,12 @@ virStateInitialize(bool privileged,
>>  return -1;
>>  
>>  for (i = 0; i < virStateDriverTabCount; i++) {
>> -if (virStateDriverTab[i]->stateInitialize) {
>> +if (virStateDriverTab[i]->stateInitialize &&
>> +!virStateDriverTab[i]->initialized) {
>>  virDrvStateInitResult ret;
>>  VIR_DEBUG("Running global init for %s state driver",
>>virStateDriverTab[i]->name);
>> +virStateDriverTab[i]->initialized = true;
>>  ret = virStateDriverTab[i]->stateInitialize(privileged,
>>  root,
>>  callback,
>> @@ -872,6 +876,7 @@ virConnectOpenInternal(const char *name,
>>  virConnectPtr ret;
>>

Re: [libvirt] [PATCH v5 1/4] Introducing new address type='unassigned' for PCI hostdevs

2019-12-17 Thread Alex Williamson

On Tue, 17 Dec 2019 16:06:44 -0300
Daniel Henrique Barboza  wrote:

> Today, to use a PCI hostdev "A" in a domain, all PCI devices
> that belongs to the same IOMMU group must also be declared in
> the domain XML, meaning that all IOMMU devices are detached
> from the host and all of them are visible to the guest.

This is not accurate.  All endpoint devices in the IOMMU group need to
be "viable" (ie. bound to vfio-pci, pci-stub, unbound) for any device
within the group to be used through vfio.  There is absolutely no
requirement that they all be declared in the XML or assigned to the
guest.  Also please clarify "IOMMU devices".  Is that all devices
within the IOMMU group?
 
> The result is that the guest will have access to all devices,
> but this is not necessarily what the administrator wanted. If only
> the hostdev "A" was intended for guest usage, but hostdevs "B" and
> "C" happened to be in the same IOMMU group of "A", the guest will
> gain access to all 3 devices. This makes the administrator rely
> on alternative solutions, such as use all hostdevs with un-managed
> mode and detached all the IOMMU before the guest starts. If
> use un-managed mode is not an option, the only alternative left is
> an ACS patch to deny guest access to "B" and "C".

Also not accurate.  The libvirt hooks interface can be used to manage
the additional devices ad-hoc, the additional devices might not need
management if they're not endpoints, they might be statically bound to
vfio-pci at boot time, etc.  I don't think the scenario is nearly as
dire as presented here.  "detached all the IOMMU" doesn't make sense.

> This patch introduces a new address type called "unassigned" to
> handle this situation where a hostdev will be owned by a domain, but
> not visible to the guest OS. This allows the administrator to
> declare all the IOMMU while also choosing which hostdevs will be

"declare all the IOMMU" doesn't make sense.

> usable by the guest. This new mechanic applies to all PCI hostdevs,
> regardless of whether they are a PCI multifunction hostdev or not.
> Using  in any case other than a PCI
> hostdev will result in error.

Regardless of my comments above, I think this support is worthwhile,
but let's not pretend there aren't solutions, this just makes it easier
to accomplish in a supported and dynamic way.  Thanks,

Alex

> Next patch will use this new address type in the QEMU driver to
> avoid adding unassigned devices to the QEMU launch command line.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  docs/schemas/domaincommon.rng |  5 ++
>  src/conf/device_conf.c|  2 +
>  src/conf/device_conf.h|  1 +
>  src/conf/domain_conf.c|  7 ++-
>  src/qemu/qemu_command.c   |  1 +
>  src/qemu/qemu_domain.c|  1 +
>  src/qemu/qemu_domain_address.c|  5 ++
>  .../hostdev-pci-address-unassigned.xml| 42 ++
>  .../hostdev-pci-address-unassigned.xml| 58 +++
>  tests/qemuxml2xmltest.c   |  1 +
>  10 files changed, 122 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
>  create mode 100644 
> tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml
> 
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index e964773f5e..5f1d4a34a4 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -5502,6 +5502,11 @@
>
>
>  
> +
> +  
> +unassigned
> +  
> +
>
>  
>
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index 4c57f0995f..4dbd5c1ac9 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress,
>"virtio-mmio",
>"isa",
>"dimm",
> +  "unassigned",
>  );
>  
>  static int
> @@ -120,6 +121,7 @@ virDomainDeviceInfoAddressIsEqual(const 
> virDomainDeviceInfo *a,
>  /* address types below don't have any specific data */
>  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
>  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
> +case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
>  break;
>  
>  case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
> diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> index d98fae775c..e091d7cfe2 100644
> --- a/src/conf/device_conf.h
> +++ b/src/conf/device_conf.h
> @@ -45,6 +45,7 @@ typedef enum {
>  VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO,
>  VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA,
>  VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM,
> +VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED,
>  
>  VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
>  } virDomainDeviceAddressType;
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e46f423b1

Re: [libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts

2019-12-17 Thread Daniel Henrique Barboza





On 12/17/19 2:58 PM, Cole Robinson wrote:

On 12/12/19 4:11 PM, Daniel Henrique Barboza wrote:

POWER hosts does not implement CPU virtualization extensions like
x86 or s390x. Instead, all bare-metal POWER hosts are considered
to be virtualization ready.

For POWER, the validation is done by checking the virtualization
kernel modules, kvm_hv and kvm_pr, to see if they are either not
installed or not loaded in the host. If the KVM modules aren't
present, we should not just warn but fail to validate.

This patch implements this support. If kvm_hv is not installed,
which can be determined by 'modinfo' returning not-zero return
code, fail the verification. If kvm_hv is installed but not
loaded, show a warning. The exception are POWER8 hosts, which can
work with kvm_pr. In its case, ACK the use of kvm_pr if kvm_hv
is not loaded/present.


For x86, we check for /dev/kvm being available and usable. This side
steps whether kvm is a module or not, in theory it could be compiled
into the kernel. Is there anything in /dev we can check for power8?


I don't know. I'll investigate.



I don't follow the reasoning for for why the module is installed vs
loaded matters for FAIL vs WARN. Can you expand on that a bit more?


The reasoning is that the module not present (i.e. not compiled
in the kernel) is not easy to solve, while the module not loaded is
a matter of loading either kvm_pr or kvm_hv.

The nonexistence of both kvm_pr and kvm_hv is something I haven't seen
in the field and I'm somewhat theorizing about it. I wouldn't oppose to
simply check for the modules being loaded and use WARN.




Rather than parsing /proc/modinfo, can we check for /sys/module/$modname
instead, or something under that directory?



Just checked. If the existence of say /sys/module/kvm_hv is an indication
that the module kvm_hv is loaded, then I see no issues with checking for
this instead of parsing modinfo.




Thanks,


DHB



- Cole



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

Re: [libvirt] [PATCH 4/4] kbase: Add document outlining backing chain XML config and troubleshooting

2019-12-17 Thread Eric Blake


On 12/17/19 12:35 PM, Peter Krempa wrote:

Signed-off-by: Peter Krempa 
---



+Image detection caveats
+---
+
+Detection of the backing chain requires libvirt to read and understand the
+``backing file`` field recorded in the image metadata and also being able to
+recurse and read the backing file. Due to security implications libvirt
+will not attempt to detect the format of the backing image if the image 
metadata
+don't contain it.


s/don't/doesn't/


+
+Libvirt also may not support a network disk storage technology and thus may be


s/also may not support/also might lack support for/


+unable to visit and detect backing chains on such storage. This may result in
+the backing chain present in the live XML to look incomplete or some operations
+not being possible. To prevent this it's possible to specify the image metadata
+explicitly in the XML.
+




+Troubleshooting
+===
+
+Few common problems which occur when managing chains of disk images.


s/Few/A few/


+
+VM refuses to start due to misconfigured backing store format
+-
+
+This problem happens on VMs where the backing chain was created manually 
outside
+of libvirt and can have multiple symptoms:
+
+- permission denied error reported on a backing image
+- ``format of backing image '%s' of image '%s' was not specified in the image 
metadata`` error reported
+- disk image looking corrupt inside the guest
+
+The cause of the above problem is that the image metadata does not record the
+format of the backing image along with the location of the image. When the
+format is not specified libvirt or qemu would have to do image format probing
+which is insecure to do as a mallicious guest could rewrite the header of the


s/mallicious/malicious/


+disk leading to access of host files. Libvirt thus does not try to assume
+the format of the backing image. The following command can be used to check if
+the image has backing image format specified:


s/has/has a/


+
+::
+
+ $ qemu-img info /tmp/copy4.qcow2
+ image: /tmp/copy4.qcow2
+ file format: qcow2
+ virtual size: 10 MiB (10485760 bytes)
+ disk size: 196 KiB
+ cluster_size: 65536
+ backing file: copy3.qcow2 (actual path: /tmp/copy3.qcow2)
+ backing file format: qcow2
+ Format specific information:
+ compat: 1.1
+ lazy refcounts: false
+ refcount bits: 16
+ corrupt: false
+
+If the ``backing file format:`` field is missing above the format was not
+specified properly. The image can be fixed by the following command:
+
+::
+
+ qemu-img rebase -f $IMAGE_FORMAT -F $BACKING_IMAGE_FORMAT -b 
$BACKING_IMAGE_PATH $IMAGE_PATH


Adding -u can make this operation faster (blindly update the image 
metadata, rather than actually crawling the entire image to perform a 
data operation in the process).  But I'm not sure whether we want to 
document that here.



+
+It is important to fill out ``$BACKING_IMAGE_FORMAT`` and ``$IMAGE_FORMAT``
+properly. ``$BACKING_IMAGE_PATH`` should be specified as a full absolute path.
+If relative referencing of the backing image is desired, the path must be
+relative to the location of image described by ``$IMAGE_PATH``.
+
+Missing images reported after after moving disk images into a different path
+
+
+The path to the backing image which is recorded in the image metadata often
+contains a full path to the backing image. This is the default libvirt-created
+image configuration. When such images are moved to a different location the
+top image will no longer point to the correct image.
+
+To fix such issue you can either fully specify the image chain in the domain 
XML
+as pointed out above or the following ``qemu-img`` command can be used:
+
+::
+
+ qemu-img rebase -u -f $IMAGE_FORMAT -F $BACKING_IMAGE_FORMAT -b 
$BACKING_IMAGE_PATH $IMAGE_PATH
+


Odd to mention -u here but not above.  Should we be consistent between 
the two examples?



+It is important to fill out ``$BACKING_IMAGE_FORMAT`` and ``$IMAGE_FORMAT``
+properly. ``$BACKING_IMAGE_PATH`` should be specified as a full absolute path.
+If relative referencing of the backing image is desired, the path must be
+relative to the location of image described by ``$IMAGE_PATH``.



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

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

[libvirt] [PATCH v5 0/4] PCI hostdev partial assignment support

2019-12-17 Thread Daniel Henrique Barboza

changes from v4 [1]:
- previous patch 3 was removed. The validation it was
implementating proved to be too restrict, while
providing no tangible benefits for the trouble of
having existing domains failing to launch. This
makes this series all about the new address type
implementation and its benefits. More info about the
rationale can be found at [1].
- documentation was changed to reflect this new
tone

[1] https://www.redhat.com/archives/libvir-list/2019-December/msg01016.html


Daniel Henrique Barboza (4):
  Introducing new address type='unassigned' for PCI hostdevs
  qemu: handle unassigned PCI hostdevs in command line
  formatdomain.html.in: document 
  news.xml: add address type='unassigned' entry

 docs/formatdomain.html.in | 13 +
 docs/news.xml | 14 +
 docs/schemas/domaincommon.rng |  5 ++
 src/conf/device_conf.c|  2 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c|  7 ++-
 src/qemu/qemu_command.c   |  5 ++
 src/qemu/qemu_domain.c|  1 +
 src/qemu/qemu_domain_address.c|  5 ++
 .../hostdev-pci-address-unassigned.args   | 31 ++
 .../hostdev-pci-address-unassigned.xml| 42 ++
 tests/qemuxml2argvtest.c  |  4 ++
 .../hostdev-pci-address-unassigned.xml| 58 +++
 tests/qemuxml2xmltest.c   |  1 +
 14 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml

-- 
2.23.0


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

[libvirt] [PATCH v5 1/4] Introducing new address type='unassigned' for PCI hostdevs

2019-12-17 Thread Daniel Henrique Barboza

Today, to use a PCI hostdev "A" in a domain, all PCI devices
that belongs to the same IOMMU group must also be declared in
the domain XML, meaning that all IOMMU devices are detached
from the host and all of them are visible to the guest.

The result is that the guest will have access to all devices,
but this is not necessarily what the administrator wanted. If only
the hostdev "A" was intended for guest usage, but hostdevs "B" and
"C" happened to be in the same IOMMU group of "A", the guest will
gain access to all 3 devices. This makes the administrator rely
on alternative solutions, such as use all hostdevs with un-managed
mode and detached all the IOMMU before the guest starts. If
use un-managed mode is not an option, the only alternative left is
an ACS patch to deny guest access to "B" and "C".

This patch introduces a new address type called "unassigned" to
handle this situation where a hostdev will be owned by a domain, but
not visible to the guest OS. This allows the administrator to
declare all the IOMMU while also choosing which hostdevs will be
usable by the guest. This new mechanic applies to all PCI hostdevs,
regardless of whether they are a PCI multifunction hostdev or not.
Using  in any case other than a PCI
hostdev will result in error.

Next patch will use this new address type in the QEMU driver to
avoid adding unassigned devices to the QEMU launch command line.

Signed-off-by: Daniel Henrique Barboza 
---
 docs/schemas/domaincommon.rng |  5 ++
 src/conf/device_conf.c|  2 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c|  7 ++-
 src/qemu/qemu_command.c   |  1 +
 src/qemu/qemu_domain.c|  1 +
 src/qemu/qemu_domain_address.c|  5 ++
 .../hostdev-pci-address-unassigned.xml| 42 ++
 .../hostdev-pci-address-unassigned.xml| 58 +++
 tests/qemuxml2xmltest.c   |  1 +
 10 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e964773f5e..5f1d4a34a4 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5502,6 +5502,11 @@
   
   
 
+
+  
+unassigned
+  
+
   
 
   
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 4c57f0995f..4dbd5c1ac9 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress,
   "virtio-mmio",
   "isa",
   "dimm",
+  "unassigned",
 );
 
 static int
@@ -120,6 +121,7 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo 
*a,
 /* address types below don't have any specific data */
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index d98fae775c..e091d7cfe2 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -45,6 +45,7 @@ typedef enum {
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO,
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA,
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM,
+VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED,
 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
 } virDomainDeviceAddressType;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e46f423b19..afa072e17d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6352,9 +6352,11 @@ virDomainHostdevDefValidate(const virDomainHostdevDef 
*hostdev)
 switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
 if (hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+hostdev->info->type != 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED &&
 hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("PCI host devices must use 'pci' address 
type"));
+   _("PCI host devices must use 'pci' or "
+ "'unassigned' address type"));
 return -1;
 }
 break;
@@ -7371,6 +7373,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
 break;
 }
@@ -7571,6 +7574,7 @@ virDomainDeviceAddres

[libvirt] [PATCH v5 2/4] qemu: handle unassigned PCI hostdevs in command line

2019-12-17 Thread Daniel Henrique Barboza

Previous patch made it possible for the QEMU driver to check if
a given PCI hostdev is unassigned, by checking if dev->info->type is
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED, meaning that this device
shouldn't be part of the actual guest launch.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_command.c   |  4 +++
 .../hostdev-pci-address-unassigned.args   | 31 +++
 tests/qemuxml2argvtest.c  |  4 +++
 3 files changed, 39 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 864967f375..ed8b878566 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5336,6 +5336,10 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
 *bootHostdevNet = 0;
 }
 
+   /* Ignore unassigned devices  */
+   if (hostdev->info->type == 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED)
+   continue;
+
 if (qemuCommandAddExtDevice(cmd, hostdev->info) < 0)
 return -1;
 
diff --git a/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args 
b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
new file mode 100644
index 00..42fae17444
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-delete \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-delete/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-delete/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-delete/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name delete \
+-S \
+-machine pc,accel=kvm,usb=off,dump-guest-core=off \
+-m 256 \
+-realtime mlock=off \
+-smp 4,sockets=4,cores=1,threads=1 \
+-uuid 583a8e8e-f0ce-4f53-89ab-092862148b25 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-delete/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=0005:90:01.2,id=hostdev2,bus=pci.0,addr=0x4 \
+-device vfio-pci,host=0005:90:01.3,id=hostdev3,bus=pci.0,addr=0x5 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 118a460633..bfbed5c31d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1332,6 +1332,10 @@ mymain(void)
 QEMU_CAPS_KVM,
 QEMU_CAPS_DEVICE_VFIO_PCI);
 
+DO_TEST("hostdev-pci-address-unassigned",
+QEMU_CAPS_KVM,
+QEMU_CAPS_DEVICE_VFIO_PCI);
+
 DO_TEST("serial-file-log",
 QEMU_CAPS_CHARDEV_FILE_APPEND,
 QEMU_CAPS_DEVICE_ISA_SERIAL,
-- 
2.23.0


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

[libvirt] [PATCH v5 4/4] news.xml: add address type='unassigned' entry

2019-12-17 Thread Daniel Henrique Barboza

Signed-off-by: Daniel Henrique Barboza 
---
 docs/news.xml | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2a25b6ca49..febda970f6 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -54,6 +54,20 @@
   written in the RST as an alternative to HTML.
 
   
+  
+
+  new PCI hostdev address type: unassigned
+
+
+  A new PCI hostdev address type 'unassigned' is introduced,
+  giving users the option to choose which PCI hostdevs
+  within the same IOMMU group will not be assigned to the
+  guest. PCI hostdevs that shouldn't be used by the guest
+  can be classified as address type='unassigned'.
+  Libvirt will still be able to manage the device as a
+  regular PCI hostdev.
+
+  
 
 
   
-- 
2.23.0


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

[libvirt] [PATCH v5 3/4] formatdomain.html.in: document

2019-12-17 Thread Daniel Henrique Barboza

Signed-off-by: Daniel Henrique Barboza 
---
 docs/formatdomain.html.in | 13 +
 1 file changed, 13 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e06cf2061b..7a5ebdd67e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4203,6 +4203,19 @@
 attributes: iobase and irq.
 Since 1.2.1
   
+  unassigned
+  For PCI hostdevs, 
+allows the admin to include a PCI hostdev in the domain XML definition,
+without making it available for the guest. This allows for 
configurations
+in which Libvirt manages the device as a regular PCI hostdev,
+regardless of whether the guest will have access to it. This is
+an alternative to scenarios in which the admin might be compelled to 
use
+an ACS patch to remove the device from the guest while Libvirt
+retains control of the PCI device.
+ is an invalid address
+type for all other device types.
+Since 6.0.0
+  
 
 
 Virtio-related options
-- 
2.23.0


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

Re: [libvirt] [PATCH 0/4] qemu: Error out if backing image format is not recorded in image metadata

2019-12-17 Thread Michal Prívozník

On 12/17/19 7:35 PM, Peter Krempa wrote:
> See patch 3/4 for explanation.
> 
> Peter Krempa (4):
>   tests: storage: Use strict version of virStorageFileGetMetadata
>   tests: storage: Remove unused test modes
>   util: storage: Don't treat files with missing backing store format as
> 'raw'
>   kbase: Add document outlining backing chain XML config and
> troubleshooting
> 
>  docs/kbase.html.in|   4 +
>  docs/kbase/backing_chains.rst | 185 ++
>  src/util/virstoragefile.c |  21 +++-
>  tests/virstoragetest.c|  42 +++-
>  4 files changed, 221 insertions(+), 31 deletions(-)
>  create mode 100644 docs/kbase/backing_chains.rst
> 

Reviewed-by: Michal Privoznik 

Michal

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

Re: [libvirt] [PATCH v2 11/12] esx: implement storagePoolListAllVolumes

2019-12-17 Thread Cole Robinson

On 11/15/19 7:40 AM, Pino Toscano wrote:
> Implement the .storagePoolListAllVolumes storage API in the esx storage
> driver, and in all its subdrivers.
> 
> Signed-off-by: Pino Toscano 
> ---
>  src/esx/esx_storage_backend_iscsi.c | 70 
>  src/esx/esx_storage_backend_vmfs.c  | 85 +
>  src/esx/esx_storage_driver.c| 19 +++
>  3 files changed, 174 insertions(+)
> 
> diff --git a/src/esx/esx_storage_backend_iscsi.c 
> b/src/esx/esx_storage_backend_iscsi.c
> index 34760a837e..2f189a92cf 100644
> --- a/src/esx/esx_storage_backend_iscsi.c
> +++ b/src/esx/esx_storage_backend_iscsi.c
> @@ -852,6 +852,75 @@ esxConnectListAllStoragePools(virConnectPtr conn,
>  
>  
>  
> +static int
> +esxStoragePoolListAllVolumes(virStoragePoolPtr pool,
> + virStorageVolPtr **vols,
> + unsigned int flags)
> +{
> +bool success = false;
> +size_t count = 0;
> +esxPrivate *priv = pool->conn->privateData;
> +esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL;
> +esxVI_HostScsiTopologyLun *hostScsiTopologyLun;
> +esxVI_ScsiLun *scsiLunList = NULL;
> +esxVI_ScsiLun *scsiLun = NULL;
> +size_t i;
> +
> +virCheckFlags(0, -1);
> +
> +if (esxVI_LookupHostScsiTopologyLunListByTargetName
> +  (priv->primary, pool->name, &hostScsiTopologyLunList) < 0) {
> +goto cleanup;
> +}
> +
> +if (!hostScsiTopologyLunList) {
> +/* iSCSI adapter may not be enabled on ESX host */
> +return 0;
> +}
> +
> +if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0)
> +goto cleanup;
> +
> +for (scsiLun = scsiLunList; scsiLun;
> + scsiLun = scsiLun->_next) {
> +for (hostScsiTopologyLun = hostScsiTopologyLunList;
> + hostScsiTopologyLun;
> + hostScsiTopologyLun = hostScsiTopologyLun->_next) {
> +if (STREQ(hostScsiTopologyLun->scsiLun, scsiLun->key)) {
> +virStorageVolPtr volume;
> +
> +volume = scsilunToStorageVol(pool->conn, scsiLun, 
> pool->name);
> +if (!volume)
> +goto cleanup;
> +
> +if (VIR_APPEND_ELEMENT(*vols, count, volume) < 0)
> +goto cleanup;
> +}
> +
> +}
> +}
> +
> +success = true;
> +
> + cleanup:
> +if (! success) {
> +if (*vols) {
> +for (i = 0; i < count; ++i)
> +VIR_FREE((*vols)[i]);
> +VIR_FREE(*vols);
> +}
> +
> +count = -1;
> +}
> +
> +esxVI_HostScsiTopologyLun_Free(&hostScsiTopologyLunList);
> +esxVI_ScsiLun_Free(&scsiLunList);
> +
> +return count;
> +}
> +
> +
> +
>  virStorageDriver esxStorageBackendISCSI = {
>  .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */
>  .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */
> @@ -873,4 +942,5 @@ virStorageDriver esxStorageBackendISCSI = {
>  .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */
>  .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */
>  .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
> +.storagePoolListAllVolumes = esxStoragePoolListAllVolumes, /* 5.10.0 */
>  };
> diff --git a/src/esx/esx_storage_backend_vmfs.c 
> b/src/esx/esx_storage_backend_vmfs.c
> index ab47a4c272..27f8b17e06 100644
> --- a/src/esx/esx_storage_backend_vmfs.c
> +++ b/src/esx/esx_storage_backend_vmfs.c
> @@ -1566,6 +1566,90 @@ esxConnectListAllStoragePools(virConnectPtr conn,
>  
>  
>  
> +static int
> +esxStoragePoolListAllVolumes(virStoragePoolPtr pool,
> + virStorageVolPtr **vols,
> + unsigned int flags)
> +{
> +bool success = false;
> +esxPrivate *priv = pool->conn->privateData;
> +esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL;
> +esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL;
> +esxVI_FileInfo *fileInfo = NULL;
> +char *directoryAndFileName = NULL;
> +size_t length;
> +size_t count = 0;
> +size_t i;
> +
> +virCheckFlags(0, -1);
> +
> +if (esxVI_LookupDatastoreContentByDatastoreName(priv->primary, 
> pool->name,
> +&searchResultsList) < 0) 
> {
> +goto cleanup;
> +}
> +
> +/* Interpret search result */
> +for (searchResults = searchResultsList; searchResults;
> + searchResults = searchResults->_next) {
> +VIR_FREE(directoryAndFileName);
> +
> +if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL,
> +   &directoryAndFileName) < 0) {
> +goto cleanup;
> +}
> +
> +/* Strip trailing separators */
> +length = strlen(directoryAndFileName);
> +
> +while (length > 0 && directoryAndFileName[length - 1] == '/') {
> +d

Re: [libvirt] [PATCH] qemu: homogenize MAC address in live & config when hotplugging a netdev

2019-12-17 Thread Michal Prívozník

On 12/17/19 6:04 PM, Laine Stump wrote:
> Prior to commit 55ce6564634 (first in libvirt 4.6.0), the XML sent to
> virDomainAttachDeviceFlags() was parsed only once, and the results of
> that parse were inserted into both the live object of the running
> domain and into the persistent config. Thus, if MAC address was
> omitted from in XML for a network device (), both the live
> and config object would have the same MAC address.
> 
> Commit 55ce6564634 changed the code to parse the incoming XML twice -
> once for live and once for config. This does eliminate the problem of
> PCI (/scsi/sata) address conflicts caused by allocating an address
> based on existing devices in live object, but then inserting the
> result into the config (which may already have a device using that
> address), BUT it also means that when the MAC address of a network
> device hasn't been specified in the XML, each copy will get a
> different auto-generated MAC address.
> 
> This results in the MAC address of the device changing the next time
> the domain is shutdown and restarted, which creates havoc with the
> guest OS's network config.
> 
> There have been several discussions about this in the last > 1 year,
> attempting to find the ideal solution to this problem that makes MAC
> addresses consistent and accounts for all sorts of corner cases with
> PCI/scsi/sata addresses. All of these discussions fizzled out because
> every proposal was either too difficult to implement or failed to fix
> some esoteric case someone thought up.
> 
> So, in the interest of solving the MAC address problem while not
> making the "other address" situation any worse than before, this patch
> simply adds a qemuDomainAttachDeviceLiveAndConfigHomogenize() function
> that (for now) copies the MAC address from the config object to the
> live object (if the original xml had  then this
> will be an effective NOP (as the macs already match)).
> 
> Any downstream libvirt containing upstream commit
> 55ce6564634 should have this patch as well.
> 
> https://bugzilla.redhat.com/1783411
> Signed-off-by: Laine Stump 
> ---
>  src/qemu/qemu_driver.c | 35 +++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 924f01d3eb..19ddff80b5 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8623,6 +8623,30 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
>  return 0;
>  }
>  
> +
> +static void
> +qemuDomainAttachDeviceLiveAndConfigHomogenize(const virDomainDeviceDef 
> *devConf,
> +  virDomainDeviceDefPtr devLive)
> +{
> +/*
> + * fixup anything that needs to be identical in the live and

s/fixup/Fixup/

> + * config versions of DeviceDef, but might not be. Do this by
> + * changing the contents of devLive.

We need to warn everybody that only "small" changes are okay. I mean, we
probably don't have to explicitly warn about not changing PCI address,
but something among these lines should do:

Remember, this is done after post parse and all other checks, be very
careful!

> + */
> +
> +/* MAC address should be identical in both DeviceDefs, but if it
> + * wasn't specified in the XML, and was instead autogenerated, it
> + * will be different for the two since they are each the result of
> + * a separate parser call. If it *was* specified, it will already
> + * be the same, so copying does no harm.
> + */
> +
> +if (devConf->type == VIR_DOMAIN_DEVICE_NET)
> +virMacAddrSet(&devLive->data.net->mac, &devConf->data.net->mac);
> +
> +}
> +
> +
>  static int
>  qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
>  virQEMUDriverPtr driver,
> @@ -8633,6 +8657,7 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
>  virDomainDefPtr vmdef = NULL;
>  g_autoptr(virQEMUDriverConfig) cfg = NULL;
>  virDomainDeviceDefPtr devConf = NULL;
> +virDomainDeviceDef devConfSave = { 0 };
>  virDomainDeviceDefPtr devLive = NULL;
>  int ret = -1;
>  unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> @@ -8657,6 +8682,13 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
>  parse_flags)))
>  goto cleanup;
>  
> +/*
> + * devConf will be NULLed out by
> + * qemuDomainAttachDeviceConfig(), so save it for later use by
> + * qemuDomainDeviceLiveAndConfigHomogenize()

This function was renamed ;-)

> + */
> +devConfSave = *devConf;
> +
>  if (virDomainDeviceValidateAliasForHotplug(vm, devConf,
> VIR_DOMAIN_AFFECT_CONFIG) 
> < 0)
>  goto cleanup;
> @@ -8678,6 +8710,9 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
>  parse_flags)))
>  goto cleanup;
>

Re: [libvirt] [PATCH v2 10/12] esx: split scsilunToStorageVol helper

2019-12-17 Thread Cole Robinson

On 11/15/19 7:40 AM, Pino Toscano wrote:
> Move the creation of a virStorageVolPtr object from the esxVI_ScsiLun
> object of a SCSI lun out of esxStorageVolLookupByName and
> esxStorageVolLookupByPath in an own helper. This way it can be used
> also in other functions.
> 
> Signed-off-by: Pino Toscano 
> ---
>  src/esx/esx_storage_backend_iscsi.c | 60 +++--
>  1 file changed, 31 insertions(+), 29 deletions(-)
> 
> diff --git a/src/esx/esx_storage_backend_iscsi.c 
> b/src/esx/esx_storage_backend_iscsi.c
> index 4f5d8e5e24..34760a837e 100644
> --- a/src/esx/esx_storage_backend_iscsi.c
> +++ b/src/esx/esx_storage_backend_iscsi.c
> @@ -440,6 +440,35 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char 
> **const names,
>  
>  
>  
> +static virStorageVolPtr
> +scsilunToStorageVol(virConnectPtr conn, esxVI_ScsiLun *scsiLun,
> +const char *pool)

I think this should be named 'scsiLunXXX' to be consistent with
capitalization used elsewhere

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH v2 09/12] esx: split datastorePathToStorageVol helper

2019-12-17 Thread Cole Robinson

On 11/15/19 7:40 AM, Pino Toscano wrote:
> Move the creation of a virStorageVolPtr object by lookup out of
> esxStorageVolLookupByPath in an own helper. This way it can be used
> also in other functions.
> 
> Signed-off-by: Pino Toscano 
> ---
>  src/esx/esx_storage_backend_vmfs.c | 28 ++--
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/src/esx/esx_storage_backend_vmfs.c 
> b/src/esx/esx_storage_backend_vmfs.c
> index 1270c21e00..ab47a4c272 100644
> --- a/src/esx/esx_storage_backend_vmfs.c
> +++ b/src/esx/esx_storage_backend_vmfs.c
> @@ -695,32 +695,40 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
>  
>  
>  
> +static virStorageVolPtr
> +datastorePathToStorageVol(virConnectPtr conn, const char *pool,
> +  const char *path)
> +{
> +esxPrivate *priv = conn->privateData;
> +g_autofree char *key = NULL;
> +
> +if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path,
> +&key) < 0) {
> +return NULL;
> +}
> +
> +return virGetStorageVol(conn, pool, path, key,
> +&esxStorageBackendVMFS, NULL);
> +}
> +
> +

Add an extra newline here, since this file uses triple line spacing

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH v2 08/12] esx: set vmfs fs type for vmfs-based datastores

2019-12-17 Thread Cole Robinson

On 11/15/19 7:40 AM, Pino Toscano wrote:
> This way they are correctly represented:
>   
> 
>   
> ... instead of 'auto'.
> 
> Signed-off-by: Pino Toscano 
> ---
>  src/esx/esx_storage_backend_vmfs.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/esx/esx_storage_backend_vmfs.c 
> b/src/esx/esx_storage_backend_vmfs.c
> index 05b273aed7..1270c21e00 100644
> --- a/src/esx/esx_storage_backend_vmfs.c
> +++ b/src/esx/esx_storage_backend_vmfs.c
> @@ -531,6 +531,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned 
> int flags)
>  }
>  } else if (esxVI_VmfsDatastoreInfo_DynamicCast(info)) {
>  def.type = VIR_STORAGE_POOL_FS;
> +def.source.format = VIR_STORAGE_POOL_FS_VMFS;
>  /*
>   * FIXME: I'm not sure how to represent the source and target of a
>   * VMFS based datastore in libvirt terms
> 

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH v2 06/12] esx: implement connectListAllNetworks

2019-12-17 Thread Cole Robinson

On 11/15/19 7:40 AM, Pino Toscano wrote:
> Implement the .connectListAllNetworks networks API in the esx network
> driver.
> 
> Signed-off-by: Pino Toscano 
> ---
>  src/esx/esx_network_driver.c | 66 
>  1 file changed, 66 insertions(+)

Needs the same treatment Jano pointed out in patch #4, size_t
adjustment, and versions changes to 6.0.0. With that:

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH v2 07/12] storage: add vmfs filesystem type

2019-12-17 Thread Cole Robinson

On 11/15/19 7:40 AM, Pino Toscano wrote:
> It will be used to represent the type of a filesystem pool in ESXi.
> 
> Signed-off-by: Pino Toscano 
> ---
>  docs/schemas/storagepool.rng  | 1 +
>  docs/schemas/storagevol.rng   | 1 +
>  docs/storage.html.in  | 3 +++
>  src/conf/storage_conf.c   | 1 +
>  src/conf/storage_conf.h   | 1 +
>  tests/storagepoolcapsschemadata/poolcaps-fs.xml   | 1 +
>  tests/storagepoolcapsschemadata/poolcaps-full.xml | 1 +
>  7 files changed, 9 insertions(+)

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH v2 05/12] esx: split virtualswitchToNetwork helper

2019-12-17 Thread Cole Robinson

On 11/15/19 7:40 AM, Pino Toscano wrote:
> Move the creation of a virNetworkPtr object from the
> esxVI_HostVirtualSwitch object of a virtual switch out of
> esxNetworkLookupByName in an own helper. This way it can be used also
> in other functions.
> 
> Signed-off-by: Pino Toscano 

Reviewed-by: Cole Robinson 

- Cole

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

[libvirt] [PATCH 4/4] kbase: Add document outlining backing chain XML config and troubleshooting

2019-12-17 Thread Peter Krempa

Signed-off-by: Peter Krempa 
---
 docs/kbase.html.in|   4 +
 docs/kbase/backing_chains.rst | 185 ++
 2 files changed, 189 insertions(+)
 create mode 100644 docs/kbase/backing_chains.rst

diff --git a/docs/kbase.html.in b/docs/kbase.html.in
index a5504a540f..c156414c41 100644
--- a/docs/kbase.html.in
+++ b/docs/kbase.html.in
@@ -25,6 +25,10 @@
 RPM deployment
 Explanation of the different RPM packages and illustration of
   which to pick for installation
+
+Backing chain 
management
+Explanation of how disk backing chain specification impacts 
libvirt's
+  behaviour and basic troubleshooting steps of disk problems.
   
 

diff --git a/docs/kbase/backing_chains.rst b/docs/kbase/backing_chains.rst
new file mode 100644
index 00..ec8f1b33b8
--- /dev/null
+++ b/docs/kbase/backing_chains.rst
@@ -0,0 +1,185 @@
+=
+Disk image chains
+=
+
+Modern disk image formats allow users to create an overlay on top of an
+existing image which will be the target of the new guest writes. This allows us
+to do snapshots of the disk state of a VM efficiently. The following text
+describes how libvirt manages such image chains and some problems which can
+occur. Note that many of the cases mentioned below are currently only relevant
+for the qemu driver.
+
+.. contents::
+
+Domain XML image and chain specification
+
+
+Disk image chains can be partially or fully configured in the domain XML. The
+basic approach is to use the  elements in the configuration.
+
+The  elements present in the live VM xml represent the actual
+topology that libvirt knows of.
+
+Basic disk setup
+
+
+Any default configuration or example usually refers only to the top (active)
+image of the backing chain.
+
+::
+
+  
+
+
+
+
+  
+
+This configuration will prompt libvirt to detect the backing image of the 
source
+image and recursively do the same thing until the end of the chain.
+
+Importance of properly setup backing chain
+--
+
+The disk image locations are used by libvirt to properly setup the security
+system used on the host so that the hypervisor can access the files and 
possibly
+also directly to configure the hypervisor to use the appropriate images. Thus
+it's important to properly setup the formats and paths of the backing images.
+
+Image detection caveats
+---
+
+Detection of the backing chain requires libvirt to read and understand the
+``backing file`` field recorded in the image metadata and also being able to
+recurse and read the backing file. Due to security implications libvirt
+will not attempt to detect the format of the backing image if the image 
metadata
+don't contain it.
+
+Libvirt also may not support a network disk storage technology and thus may be
+unable to visit and detect backing chains on such storage. This may result in
+the backing chain present in the live XML to look incomplete or some operations
+not being possible. To prevent this it's possible to specify the image metadata
+explicitly in the XML.
+
+Advanced backing chain specifications
+-
+
+To specify the topology of disk images explicitly the following XML
+specification can be used:
+
+::
+
+ 
+   
+   
+   
+ 
+ 
+ 
+   
+   
+   
+ 
+ 
+ 
+   
+   
+   
+ 
+   
+ 
+   
+   
+   
+ 
+
+This makes libvirt follow the settings as configured in the XML. Note that this
+is supported only when the 
https://libvirt.org/formatdomaincaps.html#featureBackingStoreInput
+capability is present.
+
+An empty  element signals the end of the chain. Using this
+will prevent libvirt or qemu from probing the backing chain.
+
+Note that it's also possible to partially specify the chain in the XML but omit
+the terminating element. This will result into probing from the last specified
+
+
+
+Manual image creation
+=
+
+When creating disk images manually outside of libvirt it's important to create
+them properly so that they work with libvirt as expected. The created disk
+images must contain the format of the backing image in the metadata. This
+means that the **-F** parameter of ``qemu-img`` must always be used.
+
+Troubleshooting
+===
+
+Few common problems which occur when managing chains of disk images.
+
+VM refuses to start due to misconfigured backing store format
+-
+
+This problem happens on VMs where the backing chain was created manually 
outside
+of libvirt and can have multiple symptoms:
+
+- permission denied error reported on a backing image
+- ``format of backing image '%s' of image '%s' was not specified in the image 
metadata`` error reported
+- disk image looking corr

[libvirt] [PATCH 2/4] tests: storage: Remove unused test modes

2019-12-17 Thread Peter Krempa

EXP_WARN and ALLOW_PROBE flags for the testStorageChain cases are no
longer used so we can remove them.

Signed-off-by: Peter Krempa 
---
 tests/virstoragetest.c | 30 +-
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 73717b0460..93f3d1604a 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -242,8 +242,6 @@ struct _testFileData
 enum {
 EXP_PASS = 0,
 EXP_FAIL = 1,
-EXP_WARN = 2,
-ALLOW_PROBE = 4,
 };

 struct testChainData
@@ -288,25 +286,15 @@ testStorageChain(const void *args)
 fprintf(stderr, "call should have failed\n");
 return -1;
 }
-if (data->flags & EXP_WARN) {
-if (virGetLastErrorCode() == VIR_ERR_OK) {
-fprintf(stderr, "call should have warned\n");
-return -1;
-}
-virResetLastError();
-if (virStorageFileChainGetBroken(meta, &broken) || !broken) {
-fprintf(stderr, "call should identify broken part of chain\n");
-return -1;
-}
-} else {
-if (virGetLastErrorCode()) {
-fprintf(stderr, "call should not have warned\n");
-return -1;
-}
-if (virStorageFileChainGetBroken(meta, &broken) || broken) {
-fprintf(stderr, "chain should not be identified as broken\n");
-return -1;
-}
+
+if (virGetLastErrorCode()) {
+fprintf(stderr, "call should not have reported error\n");
+return -1;
+}
+
+if (virStorageFileChainGetBroken(meta, &broken) || broken) {
+fprintf(stderr, "chain should not be identified as broken\n");
+return -1;
 }

 elt = meta;
-- 
2.23.0

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

[libvirt] [PATCH 3/4] util: storage: Don't treat files with missing backing store format as 'raw'

2019-12-17 Thread Peter Krempa

Assuming that the backing image format is raw is wrong when doing image
detection:

1) In -drive mode qemu will still probe the image format of the backing
   image. This means it will try to open a backing file of the image
   which will fail if a more advanced security model is in use.

2) In blockdev mode the image will be opened as raw actually which is
   wrong since it might be qcow. Not opening the backing images will
   also end up in the guest seeing corrupted data.

Rather than attempt to solve various corner cases when us assuming the
storage file being raw and actually being right forbid startup when the
guest image doesn't have the format specified in the metadata.

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

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 21 +
 tests/virstoragetest.c|  2 +-
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 3a3de314b8..e280a646b7 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -4981,12 +4981,25 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
src,
 goto cleanup;
 }

-if (backingFormat == VIR_STORAGE_FILE_AUTO)
+backingStore->format = backingFormat;
+
+if (backingStore->format == VIR_STORAGE_FILE_AUTO) {
+/* Assuming the backing store to be raw can lead to failures. We do
+ * it only when we must not report an error to prevent losing VMs.
+ * Otherwise report an error.
+ */
+if (report_broken) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("format of backing image '%s' of image '%s' 
was not specified in the image metadata"),
+   src->backingStoreRaw, NULLSTR(src->path));
+return -1;
+}
+
 backingStore->format = VIR_STORAGE_FILE_RAW;
-else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
+}
+
+if (backingStore->format == VIR_STORAGE_FILE_AUTO_SAFE)
 backingStore->format = VIR_STORAGE_FILE_AUTO;
-else
-backingStore->format = backingFormat;

 if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent,
 uid, gid,
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 93f3d1604a..b9d4a45cdd 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -751,7 +751,7 @@ mymain(void)
 .format = VIR_STORAGE_FILE_QCOW2,
 };
 TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2,
-   (&wrap_as_raw, &qcow2_as_raw), EXP_PASS);
+   (&wrap_as_raw, &qcow2_as_raw), EXP_FAIL);

 /* Rewrite qcow2 to a missing backing file, with backing type */
 virCommandFree(cmd);
-- 
2.23.0

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

[libvirt] [PATCH 0/4] qemu: Error out if backing image format is not recorded in image metadata

2019-12-17 Thread Peter Krempa

See patch 3/4 for explanation.

Peter Krempa (4):
  tests: storage: Use strict version of virStorageFileGetMetadata
  tests: storage: Remove unused test modes
  util: storage: Don't treat files with missing backing store format as
'raw'
  kbase: Add document outlining backing chain XML config and
troubleshooting

 docs/kbase.html.in|   4 +
 docs/kbase/backing_chains.rst | 185 ++
 src/util/virstoragefile.c |  21 +++-
 tests/virstoragetest.c|  42 +++-
 4 files changed, 221 insertions(+), 31 deletions(-)
 create mode 100644 docs/kbase/backing_chains.rst

-- 
2.23.0

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

[libvirt] [PATCH 1/4] tests: storage: Use strict version of virStorageFileGetMetadata

2019-12-17 Thread Peter Krempa

Pass in 'true' as '@report_broken' of virStorageFileGetMetadata to make
it fail in the tests. The most important code paths (when starting the
VM) expect this function to fail rather than silently return partial
data. Switch the test to exercise this more important code path.

Signed-off-by: Peter Krempa 
---
 tests/virstoragetest.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 407378bab4..73717b0460 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -103,7 +103,7 @@ testStorageFileGetMetadata(const char *path,

 def->path = g_strdup(path);

-if (virStorageFileGetMetadata(def, uid, gid, false) < 0)
+if (virStorageFileGetMetadata(def, uid, gid, true) < 0)
 return NULL;

 return g_steal_pointer(&def);
@@ -775,7 +775,7 @@ mymain(void)
 qcow2.expBackingStoreRaw = datadir "/bogus";

 /* Qcow2 file with missing backing file but specified type */
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN);
+TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_FAIL);

 /* Rewrite qcow2 to a missing backing file, without backing type */
 virCommandFree(cmd);
@@ -785,7 +785,7 @@ mymain(void)
 ret = -1;

 /* Qcow2 file with missing backing file and no specified type */
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN);
+TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_FAIL);

 /* Rewrite qcow2 to use an nbd: protocol as backend */
 virCommandFree(cmd);
@@ -916,7 +916,7 @@ mymain(void)
 qcow2.expBackingStoreRaw = "qcow2";

 /* Behavior of an infinite loop chain */
-TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_WARN);
+TEST_CHAIN(absqcow2, VIR_STORAGE_FILE_QCOW2, (&qcow2), EXP_FAIL);

 /* Rewrite wrap and qcow2 to be mutually-referential loop */
 virCommandFree(cmd);
@@ -933,7 +933,7 @@ mymain(void)
 qcow2.expBackingStoreRaw = "wrap";

 /* Behavior of an infinite loop chain */
-TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2), EXP_WARN);
+TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, (&wrap, &qcow2), EXP_FAIL);

 /* Rewrite qcow2 to use an rbd: protocol as backend */
 virCommandFree(cmd);
-- 
2.23.0

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

Re: [libvirt] [PATCH-for-4.2] hw/mips: Deprecate the r4k machine

2019-12-17 Thread Thomas Huth

 Hi,

On 25/11/2019 11.41, Philippe Mathieu-Daudé wrote:
[...]
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 4b4b7425ac..05265b43c8 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -266,6 +266,11 @@ The 'scsi-disk' device is deprecated. Users should use 
> 'scsi-hd' or
>  
>  @section System emulator machines
>  
> +@subsection mips r4k platform (since 4.2)

Since the patch has now been merged after the release of 4.2, the mips
4k platform will be deprecated in 5.0 instead. Could you send a patch to
fix it up?

 Thanks,
  Thomas

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

Re: [libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts

2019-12-17 Thread Cole Robinson

On 12/12/19 4:11 PM, Daniel Henrique Barboza wrote:
> POWER hosts does not implement CPU virtualization extensions like
> x86 or s390x. Instead, all bare-metal POWER hosts are considered
> to be virtualization ready.
> 
> For POWER, the validation is done by checking the virtualization
> kernel modules, kvm_hv and kvm_pr, to see if they are either not
> installed or not loaded in the host. If the KVM modules aren't
> present, we should not just warn but fail to validate.
> 
> This patch implements this support. If kvm_hv is not installed,
> which can be determined by 'modinfo' returning not-zero return
> code, fail the verification. If kvm_hv is installed but not
> loaded, show a warning. The exception are POWER8 hosts, which can
> work with kvm_pr. In its case, ACK the use of kvm_pr if kvm_hv
> is not loaded/present.

For x86, we check for /dev/kvm being available and usable. This side
steps whether kvm is a module or not, in theory it could be compiled
into the kernel. Is there anything in /dev we can check for power8?

I don't follow the reasoning for for why the module is installed vs
loaded matters for FAIL vs WARN. Can you expand on that a bit more?

Rather than parsing /proc/modinfo, can we check for /sys/module/$modname
instead, or something under that directory?

- Cole

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

Re: [libvirt] [PATCH 4/7] libvirt: support an "embed" URI path selector for opening drivers

2019-12-17 Thread Cole Robinson

On 12/2/19 10:03 AM, Daniel P. Berrangé wrote:
> The driver URI scheme:
> 
>   "$drivername:///embed?root=/some/path"
> 
> enables a new way to use the drivers by embedding them directly in the
> calling process. To use this the process must have a thread running the
> libvirt event loop. This URI will then cause libvirt to dynamically load
> the driver module and call its global initialization function. This
> syntax is applicable to any driver, but only those will have been
> modified to support a custom root directory and embed URI path will
> successfully open.
> 
> The application can now make normal libvirt API calls which are all
> serviced in-process with no RPC layer involved.
> 
> It is required to specify an explicit root directory, and locks will be
> acquired on this directory to avoid conflicting with another app that
> might accidentally pick the same directory.
> 
> Use of '/' is not explicitly forbidden, but note that the file layout
> used underneath the embedded driver root does not match the file
> layout used by system/session mode drivers. So this cannot be used as
> a backdoor to interact with, or fake, the system/session mode drivers.
> 
> Libvirt will create arbitrary files underneath this root directory. The
> root directory can be kept untouched across connection open attempts if
> the application needs persistence. The application is responsible for
> purging everything underneath this root directory when finally no longer
> required.
> 
> Even when a virt driver is used in embedded mode, it is still possible
> for it to in turn use functionality that calls out to other secondary
> drivers in libvirtd. For example an embedded instance of QEMU can open
> the network, secret or storage drivers in the system libvirtd.
> 
> That said, the application would typically want to at least open an
> embedded secret driver ("secret:///embed?root=/some/path"). Note that
> multiple different embedded drivers can use the same root prefix and
> co-operate just as they would inside a normal libvirtd daemon.
> 
> A key thing to note is that for this to work, the application that links
> to libvirt *MUST* be built with -Wl,--export-dynamic to ensure that
> symbols from libvirt.so are exported & thus available to the dynamically
> loaded driver module. If libvirt.so itself was dynamically loaded then
> RTLD_GLOBAL must be passed to dlopen().
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/driver-state.h |  1 +
>  src/driver.h   |  2 ++
>  src/libvirt.c  | 72 --
>  3 files changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/src/driver-state.h b/src/driver-state.h
> index 1e2f6ed247..6b3f501e05 100644
> --- a/src/driver-state.h
> +++ b/src/driver-state.h
> @@ -50,6 +50,7 @@ typedef virStateDriver *virStateDriverPtr;
>  
>  struct _virStateDriver {
>  const char *name;
> +bool initialized;
>  virDrvStateInitialize stateInitialize;
>  virDrvStateCleanup stateCleanup;
>  virDrvStateReload stateReload;
> diff --git a/src/driver.h b/src/driver.h
> index ca82ac974b..6278aa05b3 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -82,6 +82,8 @@ struct _virConnectDriver {
>  bool localOnly;
>  /* Whether driver needs a server in the URI */
>  bool remoteOnly;
> +/* Whether driver can be used in embedded mode */
> +bool embeddable;
>  /*
>   * NULL terminated list of supported URI schemes.
>   *  - Single element { NULL } list indicates no supported schemes
> diff --git a/src/libvirt.c b/src/libvirt.c
> index bd2952d036..17b6506faa 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -52,6 +52,7 @@
>  # include "rpc/virnettlscontext.h"
>  #endif
>  #include "vircommand.h"
> +#include "virevent.h"
>  #include "virfile.h"
>  #include "virrandom.h"
>  #include "viruri.h"
> @@ -84,6 +85,7 @@
>  #ifdef WITH_BHYVE
>  # include "bhyve/bhyve_driver.h"
>  #endif
> +#include "access/viraccessmanager.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
> @@ -676,10 +678,12 @@ virStateInitialize(bool privileged,
>  return -1;
>  
>  for (i = 0; i < virStateDriverTabCount; i++) {
> -if (virStateDriverTab[i]->stateInitialize) {
> +if (virStateDriverTab[i]->stateInitialize &&
> +!virStateDriverTab[i]->initialized) {
>  virDrvStateInitResult ret;
>  VIR_DEBUG("Running global init for %s state driver",
>virStateDriverTab[i]->name);
> +virStateDriverTab[i]->initialized = true;
>  ret = virStateDriverTab[i]->stateInitialize(privileged,
>  root,
>  callback,
> @@ -872,6 +876,7 @@ virConnectOpenInternal(const char *name,
>  virConnectPtr ret;
>  g_autoptr(virConf) conf = NULL;
>  char *uristr = NULL;
> +bool embed = false;
>  
>  ret = virGetConnect();
>  if (ret == NULL

Re: [libvirt] [PATCH 3/7] event: add API for requiring an event loop impl to be registered

2019-12-17 Thread Cole Robinson

On 12/2/19 10:03 AM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  po/POTFILES.in  |  1 +
>  src/util/virevent.c | 25 +
>  src/util/virevent.h |  2 ++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index debb51cd70..b396797ff2 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -238,6 +238,7 @@
>  @SRCDIR@/src/util/virdnsmasq.c
>  @SRCDIR@/src/util/virerror.c
>  @SRCDIR@/src/util/virerror.h
> +@SRCDIR@/src/util/virevent.c
>  @SRCDIR@/src/util/vireventpoll.c
>  @SRCDIR@/src/util/virfcp.c
>  @SRCDIR@/src/util/virfdstream.c
> diff --git a/src/util/virevent.c b/src/util/virevent.c
> index 3cac9f9472..a86acf64c0 100644
> --- a/src/util/virevent.c
> +++ b/src/util/virevent.c
> @@ -29,6 +29,9 @@
>  
>  VIR_LOG_INIT("util.event");
>  
> +
> +#define VIR_FROM_THIS VIR_FROM_EVENT
> +
>  static virEventAddHandleFunc addHandleImpl;
>  static virEventUpdateHandleFunc updateHandleImpl;
>  static virEventRemoveHandleFunc removeHandleImpl;
> @@ -251,6 +254,26 @@ void virEventRegisterImpl(virEventAddHandleFunc 
> addHandle,
>  removeTimeoutImpl = removeTimeout;
>  }
>  
> +
> +/**
> + * virEventRequireImpl:
> + *
> + * Require that there is an event loop implementation
> + * registered.
> + *
> + * Returns: -1 if no event loop is registered, 0 otherwise
> + */
> +int virEventRequireImpl(void)
> +{
> +if (!addHandleImpl || !addTimeoutImpl) {
> +virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +   _("An event loop implementation must be registered"));
> +return -1;
> +}
> +
> +return 0;
> +}
> +

I just noticed, maybe VIR_ERR_NO_SUPPORT isn't the best thing here,
since this is more a client config issue if it didn't register an event
loop impl

- Cole

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

[libvirt] [PATCH 1/2] qemu_firmware: Pass virDomainDef into qemuFirmwareFillDomain()

2019-12-17 Thread Michal Privoznik

This function needs domain definition really, we don't need to
pass the whole domain object. This saves couple of dereferences
and characters esp. in more checks to come.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_firmware.c | 12 ++--
 src/qemu/qemu_firmware.h |  2 +-
 src/qemu/qemu_process.c  |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index f62ce90ac9..96058c9b45 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1204,7 +1204,7 @@ qemuFirmwareFetchParsedConfigs(bool privileged,
 
 int
 qemuFirmwareFillDomain(virQEMUDriverPtr driver,
-   virDomainObjPtr vm,
+   virDomainDefPtr def,
unsigned int flags)
 {
 VIR_AUTOSTRINGLIST paths = NULL;
@@ -1217,7 +1217,7 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
 if (!(flags & VIR_QEMU_PROCESS_START_NEW))
 return 0;
 
-if (vm->def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
+if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
 return 0;
 
 if ((nfirmwares = qemuFirmwareFetchParsedConfigs(driver->privileged,
@@ -1225,7 +1225,7 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
 return -1;
 
 for (i = 0; i < nfirmwares; i++) {
-if (qemuFirmwareMatchDomain(vm->def, firmwares[i], paths[i])) {
+if (qemuFirmwareMatchDomain(def, firmwares[i], paths[i])) {
 theone = firmwares[i];
 VIR_DEBUG("Found matching firmware (description path '%s')",
   paths[i]);
@@ -1236,7 +1236,7 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
 if (!theone) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("Unable to find any firmware to satisfy '%s'"),
-   
virDomainOsDefFirmwareTypeToString(vm->def->os.firmware));
+   virDomainOsDefFirmwareTypeToString(def->os.firmware));
 goto cleanup;
 }
 
@@ -1245,10 +1245,10 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
  * likely that admin/FW manufacturer messed up. */
 qemuFirmwareSanityCheck(theone, paths[i]);
 
-if (qemuFirmwareEnableFeatures(driver, vm->def, theone) < 0)
+if (qemuFirmwareEnableFeatures(driver, def, theone) < 0)
 goto cleanup;
 
-vm->def->os.firmware = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
+def->os.firmware = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
 
 ret = 0;
  cleanup:
diff --git a/src/qemu/qemu_firmware.h b/src/qemu/qemu_firmware.h
index 4be65bc664..37cbfae39d 100644
--- a/src/qemu/qemu_firmware.h
+++ b/src/qemu/qemu_firmware.h
@@ -45,7 +45,7 @@ qemuFirmwareFetchConfigs(char ***firmwares,
 
 int
 qemuFirmwareFillDomain(virQEMUDriverPtr driver,
-   virDomainObjPtr vm,
+   virDomainDefPtr def,
unsigned int flags);
 
 int
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7e1db50e8f..6d4511a49f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6344,7 +6344,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
 goto cleanup;
 
 VIR_DEBUG("Prepare bios/uefi paths");
-if (qemuFirmwareFillDomain(driver, vm, flags) < 0)
+if (qemuFirmwareFillDomain(driver, vm->def, flags) < 0)
 goto cleanup;
 if (qemuDomainInitializePflashStorageSource(vm) < 0)
 goto cleanup;
-- 
2.24.1

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

[libvirt] [PATCH 0/2] qemu_firmware: Try to autofill for old style UEFI specification

2019-12-17 Thread Michal Privoznik

See 2/2 for info.

Michal Prívozník (2):
  qemu_firmware: Pass virDomainDef into qemuFirmwareFillDomain()
  qemu_firmware: Try to autofill for old style UEFI specification

 src/qemu/qemu_firmware.c | 90 +---
 src/qemu/qemu_firmware.h |  2 +-
 src/qemu/qemu_process.c  |  2 +-
 3 files changed, 78 insertions(+), 16 deletions(-)

-- 
2.24.1

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

Re: [libvirt] [PATCH 3/7] event: add API for requiring an event loop impl to be registered

2019-12-17 Thread Cole Robinson

On 12/2/19 10:03 AM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  po/POTFILES.in  |  1 +
>  src/util/virevent.c | 25 +
>  src/util/virevent.h |  2 ++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index debb51cd70..b396797ff2 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -238,6 +238,7 @@
>  @SRCDIR@/src/util/virdnsmasq.c
>  @SRCDIR@/src/util/virerror.c
>  @SRCDIR@/src/util/virerror.h
> +@SRCDIR@/src/util/virevent.c
>  @SRCDIR@/src/util/vireventpoll.c
>  @SRCDIR@/src/util/virfcp.c
>  @SRCDIR@/src/util/virfdstream.c
> diff --git a/src/util/virevent.c b/src/util/virevent.c
> index 3cac9f9472..a86acf64c0 100644
> --- a/src/util/virevent.c
> +++ b/src/util/virevent.c
> @@ -29,6 +29,9 @@
>  
>  VIR_LOG_INIT("util.event");
>  
> +
> +#define VIR_FROM_THIS VIR_FROM_EVENT
> +

Other files I looked at, this comes before VIR_LOG_INIT, but I don't
think that has any effect

>  static virEventAddHandleFunc addHandleImpl;
>  static virEventUpdateHandleFunc updateHandleImpl;
>  static virEventRemoveHandleFunc removeHandleImpl;
> @@ -251,6 +254,26 @@ void virEventRegisterImpl(virEventAddHandleFunc 
> addHandle,
>  removeTimeoutImpl = removeTimeout;
>  }
>  
> +
> +/**
> + * virEventRequireImpl:
> + *
> + * Require that there is an event loop implementation
> + * registered.
> + *
> + * Returns: -1 if no event loop is registered, 0 otherwise
> + */
> +int virEventRequireImpl(void)
> +{
> +if (!addHandleImpl || !addTimeoutImpl) {
> +virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +   _("An event loop implementation must be registered"));
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> /**

Spacing is inconsistent here, there's two lines before the function and
one after

Reviewed-by: Cole Robinson 

- Cole

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

[libvirt] [PATCH 2/2] qemu_firmware: Try to autofill for old style UEFI specification

2019-12-17 Thread Michal Privoznik

While we discourage people to use the old style of specifying
UEFI for their domains (the old style is putting path to the FW
image under /domain/os/loader/ whilst the new one is using
/domain/os/@firmware), some applications might have not adopted
yet. They still rely on libvirt autofilling NVRAM path and
figuring out NVRAM template when using the old way (notably
virt-install does this). And in a way they are right. However,
since we really want distro maintainers to leave
--with-loader-nvram configure option and rely on JSON
descriptors, we need to implement autofilling of NVRAM template
for the old way too.

Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1782778
RHEL: https://bugzilla.redhat.com/show_bug.cgi?id=1776949

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_firmware.c | 82 +++-
 1 file changed, 72 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 96058c9b45..a31bde5d04 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -935,17 +935,53 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
 const qemuFirmware *fw,
 const char *path)
 {
-size_t i;
+qemuFirmwareOSInterface want = QEMU_FIRMWARE_OS_INTERFACE_NONE;
 bool supportsS3 = false;
 bool supportsS4 = false;
 bool requiresSMM = false;
 bool supportsSEV = false;
+size_t i;
+
+switch (def->os.firmware) {
+case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
+want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+break;
+case VIR_DOMAIN_OS_DEF_FIRMWARE_EFI:
+want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+break;
+case VIR_DOMAIN_OS_DEF_FIRMWARE_NONE:
+case VIR_DOMAIN_OS_DEF_FIRMWARE_LAST:
+break;
+}
+
+if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE &&
+def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE &&
+def->os.loader) {
+switch (def->os.loader->type) {
+case VIR_DOMAIN_LOADER_TYPE_ROM:
+want = QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+break;
+case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+want = QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+break;
+case VIR_DOMAIN_LOADER_TYPE_NONE:
+case VIR_DOMAIN_LOADER_TYPE_LAST:
+break;
+}
+
+if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH ||
+STRNEQ(def->os.loader->path, 
fw->mapping.data.flash.executable.filename)) {
+VIR_DEBUG("Not matching FW interface %s or loader "
+  "path '%s' for user provided path '%s'",
+  qemuFirmwareDeviceTypeToString(fw->mapping.device),
+  fw->mapping.data.flash.executable.filename,
+  def->os.loader->path);
+return false;
+}
+}
 
 for (i = 0; i < fw->ninterfaces; i++) {
-if ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS &&
- fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_BIOS) ||
-(def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI &&
- fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_UEFI))
+if (fw->interfaces[i] == want)
 break;
 }
 
@@ -1211,14 +1247,33 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
 qemuFirmwarePtr *firmwares = NULL;
 ssize_t nfirmwares = 0;
 const qemuFirmware *theone = NULL;
+bool needResult = true;
 size_t i;
 int ret = -1;
 
 if (!(flags & VIR_QEMU_PROCESS_START_NEW))
 return 0;
 
-if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
-return 0;
+/* Fill in FW paths if either os.firmware is enabled, or
+ * loader path was provided with no nvram varstore. */
+if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
+/* This is horrific check, but loosely said, if UEFI
+ * image was provided by the old method (by specifying
+ * its path in domain XML) but no template for NVRAM was
+ * specified ... */
+if (!(def->os.loader &&
+  def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
+  def->os.loader->path &&
+  def->os.loader->readonly == VIR_TRISTATE_BOOL_YES &&
+  !def->os.loader->templt &&
+  def->os.loader->nvram &&
+  !virFileExists(def->os.loader->nvram)))
+return 0;
+
+/* ... then we want to consult JSON FW descriptors first,
+ * but we don't want to fail if we haven't found a match. */
+needResult = false;
+}
 
 if ((nfirmwares = qemuFirmwareFetchParsedConfigs(driver->privileged,
  &firmwares, &paths)) < 0)
@@ -1234,9 +1289,16 @@ qemuFirmwareFillDomain(virQEMUDriverPtr driver,
 }
 
 if (!theone) {
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("Unable to find any firmware to satisfy '%s'"),
-

Re: [libvirt] [PATCH 2/7] libvirt: pass a directory path into drivers for embedded usage

2019-12-17 Thread Cole Robinson

On 12/2/19 10:03 AM, Daniel P. Berrangé wrote:
> The intent here is to allow the virt drivers to be run directly embedded
> in an arbitrary process without interfering with libvirtd. To achieve
> this they need to store all their configuration & state in a separate
> directory tree from the main system or session libvirtd instances.
> 
> This can be useful for doing testing of the virt drivers in "make check"
> without interfering with the user's own libvirtd instances.
> 
> It can also be used for applications using KVM/QEMU as a piece of
> infrastructure to build an service, rather than for general purpose
> OS hosting. A long standing example is libguestfs, which would prefer
> if its temporary VMs did show up in the main libvirtd VM list, because
> this confuses apps such as OpenStack Nova. A more recent example would
> be Kata which is using KVM as a technology to build containers.
> 
> Signed-off-by: Daniel P. Berrangé 

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH 1/7] access: report an error if no access manager is present

2019-12-17 Thread Cole Robinson

On 12/2/19 10:03 AM, Daniel P. Berrangé wrote:
> The code calling this method expects it to have reported an error on
> failure.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/access/viraccessmanager.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
> index 31e1787919..f4120c6139 100644
> --- a/src/access/viraccessmanager.c
> +++ b/src/access/viraccessmanager.c
> @@ -65,6 +65,11 @@ VIR_ONCE_GLOBAL_INIT(virAccessManager);
>  
>  virAccessManagerPtr virAccessManagerGetDefault(void)
>  {
> +if (virAccessManagerDefault == NULL) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("No access manager registered"));
> +return NULL;
> +}

I would expect to see a newline here

>  return virObjectRef(virAccessManagerDefault);
>  }
>  
> 

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-17 Thread Daniel Henrique Barboza





On 12/17/19 1:43 PM, Daniel Henrique Barboza wrote:

I don't actually recall saying that :-). I haven't looked in the list
archives, but what I *can* imagine myself saying is that only devices
mentioned in the XML should be manipulated in any way by libvirt. So,


+1

for example, you shouldn't unbind device X from its host driver if there
is nothing in the XML telling you to do that. But if a device isn't
mentioned in the XML, and is already bound to some driver that is
acceptable to the VFIO subsystem (e.g. vfio-pci, pci-stub or no driver
at all (? is that right Alex?)) then that should not create any problem.


Yes, that's right.


Doing otherwise would break too many existing configs. (For example, my
own assigned-GPU config, which assumes that all the devices are already
bound to the proper driver, and uses "managed='no'")



I am re-reading this info and reassessing what I intended to do. Suppose a
scenario in which host IOMMU has PCI devices A,B and C. Let's also suppose
that all of them are already bind to vfio-pci.

If I create a guest with A and B as PCI hostdevs, with managed=yes, using
vfio-pci, the setup would work as it is. If I put the IOMMU validation I was
planning to, it will stop working because "C" isn't described in the domain
XML.

If we stick with the directive "Libvirt shouldn't tamper with devices that
are not described in the domain XML" that Laine mentioned up there, and this
directive alone, then I can't make such assumptions about whether the user
will be OK with assigning everything to vfio-pci, given that there are other
acceptable alternatives for the VFIO subsystem that aren't restricted to
that.

I'm convinced that this validation I was pursuing here is a bad idea. It
has a great potential of being annoying, making assumptions about real life
configurations that aren't true, and offering not much in return. I'll
remove it from the series. The result is that the user will have a
new option to let Libvirt control all the IOMMU devices, making some
of the unassignable to the guest. But it will be a new option, not a
new rule that all existing domains will need to adhere to.


Thanks,


DHB


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

[libvirt] [PATCH] qemu: homogenize MAC address in live & config when hotplugging a netdev

2019-12-17 Thread Laine Stump

Prior to commit 55ce6564634 (first in libvirt 4.6.0), the XML sent to
virDomainAttachDeviceFlags() was parsed only once, and the results of
that parse were inserted into both the live object of the running
domain and into the persistent config. Thus, if MAC address was
omitted from in XML for a network device (), both the live
and config object would have the same MAC address.

Commit 55ce6564634 changed the code to parse the incoming XML twice -
once for live and once for config. This does eliminate the problem of
PCI (/scsi/sata) address conflicts caused by allocating an address
based on existing devices in live object, but then inserting the
result into the config (which may already have a device using that
address), BUT it also means that when the MAC address of a network
device hasn't been specified in the XML, each copy will get a
different auto-generated MAC address.

This results in the MAC address of the device changing the next time
the domain is shutdown and restarted, which creates havoc with the
guest OS's network config.

There have been several discussions about this in the last > 1 year,
attempting to find the ideal solution to this problem that makes MAC
addresses consistent and accounts for all sorts of corner cases with
PCI/scsi/sata addresses. All of these discussions fizzled out because
every proposal was either too difficult to implement or failed to fix
some esoteric case someone thought up.

So, in the interest of solving the MAC address problem while not
making the "other address" situation any worse than before, this patch
simply adds a qemuDomainAttachDeviceLiveAndConfigHomogenize() function
that (for now) copies the MAC address from the config object to the
live object (if the original xml had  then this
will be an effective NOP (as the macs already match)).

Any downstream libvirt containing upstream commit
55ce6564634 should have this patch as well.

https://bugzilla.redhat.com/1783411
Signed-off-by: Laine Stump 
---
 src/qemu/qemu_driver.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 924f01d3eb..19ddff80b5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8623,6 +8623,30 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 return 0;
 }
 
+
+static void
+qemuDomainAttachDeviceLiveAndConfigHomogenize(const virDomainDeviceDef 
*devConf,
+  virDomainDeviceDefPtr devLive)
+{
+/*
+ * fixup anything that needs to be identical in the live and
+ * config versions of DeviceDef, but might not be. Do this by
+ * changing the contents of devLive.
+ */
+
+/* MAC address should be identical in both DeviceDefs, but if it
+ * wasn't specified in the XML, and was instead autogenerated, it
+ * will be different for the two since they are each the result of
+ * a separate parser call. If it *was* specified, it will already
+ * be the same, so copying does no harm.
+ */
+
+if (devConf->type == VIR_DOMAIN_DEVICE_NET)
+virMacAddrSet(&devLive->data.net->mac, &devConf->data.net->mac);
+
+}
+
+
 static int
 qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 virQEMUDriverPtr driver,
@@ -8633,6 +8657,7 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 virDomainDefPtr vmdef = NULL;
 g_autoptr(virQEMUDriverConfig) cfg = NULL;
 virDomainDeviceDefPtr devConf = NULL;
+virDomainDeviceDef devConfSave = { 0 };
 virDomainDeviceDefPtr devLive = NULL;
 int ret = -1;
 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
@@ -8657,6 +8682,13 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 parse_flags)))
 goto cleanup;
 
+/*
+ * devConf will be NULLed out by
+ * qemuDomainAttachDeviceConfig(), so save it for later use by
+ * qemuDomainDeviceLiveAndConfigHomogenize()
+ */
+devConfSave = *devConf;
+
 if (virDomainDeviceValidateAliasForHotplug(vm, devConf,
VIR_DOMAIN_AFFECT_CONFIG) < 
0)
 goto cleanup;
@@ -8678,6 +8710,9 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
 parse_flags)))
 goto cleanup;
 
+if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+qemuDomainAttachDeviceLiveAndConfigHomogenize(&devConfSave, 
devLive);
+
 if (virDomainDeviceValidateAliasForHotplug(vm, devLive,
VIR_DOMAIN_AFFECT_LIVE) < 0)
 goto cleanup;
-- 
2.23.0

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

Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-17 Thread Alex Williamson

On Tue, 17 Dec 2019 13:43:14 -0300
Daniel Henrique Barboza  wrote:

> On 12/17/19 1:32 PM, Alex Williamson wrote:
> > On Tue, 17 Dec 2019 11:25:38 -0500
> > Laine Stump  wrote:
> >   
> >> On 12/16/19 6:03 PM, Daniel Henrique Barboza wrote:  
> >>> About breaking existing configurations, there is the possibility of not
> >>> going forward with patch 03, which is enforcing this rule of declaring
> >>> all the
> >>> IOMMU group. Existing domains will keep working as usual, the option to
> >>> unassign devices will still be present, but the user will have to deal
> >>> with
> >>> the potential QEMU errors if not all PCI devices were detached from
> >>> the host.
> >>>
> >>> In this case, the 'unassigned' type will become more of a ON/OFF
> >>> switch to
> >>> add/remove the PCI hostdev from the guest without removing it from the
> >>> domain XML. It is still useful, but we lose the idea of all the IOMMU
> >>> devices being described in the domain XML, which is something Laine
> >>> mentioned it would be desirable in one of the RFCs.  
> >>
> >>
> >> I don't actually recall saying that :-). I haven't looked in the list
> >> archives, but what I *can* imagine myself saying is that only devices
> >> mentioned in the XML should be manipulated in any way by libvirt. So,  
> > 
> > +1
> > 
> >> for example, you shouldn't unbind device X from its host driver if there
> >> is nothing in the XML telling you to do that. But if a device isn't
> >> mentioned in the XML, and is already bound to some driver that is
> >> acceptable to the VFIO subsystem (e.g. vfio-pci, pci-stub or no driver
> >> at all (? is that right Alex?)) then that should not create any problem.  
> > 
> > Yes, that's right.
> >   
> >> Doing otherwise would break too many existing configs. (For example, my
> >> own assigned-GPU config, which assumes that all the devices are already
> >> bound to the proper driver, and uses "managed='no'")  
> 
> 
> This is the new approach of the series I implemented today and plan to
> to send for review today/tomorrow. I realized, after all the discussions
> yesterday with Alex, that the patch series would be best just sticking with
> what we want fixed (managed=yes and parcial assignment) and leaving
> unmanaged (managed=no) configurations alone. If the user has an existing,
> working unmanaged setup, this means that the user chose to manage device
> detach/re-attach manually and shouldn't be bothered with a change that's
> aimed to managed devices.

There are of course existing, working managed=yes configurations where
the set of assigned devices is only a subset of the IOMMU group, and
the user has configured other means to make the group viable relative
to vfio.  The statement above doesn't convince me that the next
iteration isn't simply going to restrict its manipulation of other
devices.  As Laine says above and I said in my previous reply, libvirt
should not manipulate the driver binding of any devices not explicitly
listed in the domain XMl.  Thanks,

Alex

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

Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-17 Thread Daniel Henrique Barboza





On 12/17/19 1:32 PM, Alex Williamson wrote:

On Tue, 17 Dec 2019 11:25:38 -0500
Laine Stump  wrote:


On 12/16/19 6:03 PM, Daniel Henrique Barboza wrote:

About breaking existing configurations, there is the possibility of not
going forward with patch 03, which is enforcing this rule of declaring
all the
IOMMU group. Existing domains will keep working as usual, the option to
unassign devices will still be present, but the user will have to deal
with
the potential QEMU errors if not all PCI devices were detached from
the host.

In this case, the 'unassigned' type will become more of a ON/OFF
switch to
add/remove the PCI hostdev from the guest without removing it from the
domain XML. It is still useful, but we lose the idea of all the IOMMU
devices being described in the domain XML, which is something Laine
mentioned it would be desirable in one of the RFCs.



I don't actually recall saying that :-). I haven't looked in the list
archives, but what I *can* imagine myself saying is that only devices
mentioned in the XML should be manipulated in any way by libvirt. So,


+1
  

for example, you shouldn't unbind device X from its host driver if there
is nothing in the XML telling you to do that. But if a device isn't
mentioned in the XML, and is already bound to some driver that is
acceptable to the VFIO subsystem (e.g. vfio-pci, pci-stub or no driver
at all (? is that right Alex?)) then that should not create any problem.


Yes, that's right.


Doing otherwise would break too many existing configs. (For example, my
own assigned-GPU config, which assumes that all the devices are already
bound to the proper driver, and uses "managed='no'")



This is the new approach of the series I implemented today and plan to
to send for review today/tomorrow. I realized, after all the discussions
yesterday with Alex, that the patch series would be best just sticking with
what we want fixed (managed=yes and parcial assignment) and leaving
unmanaged (managed=no) configurations alone. If the user has an existing,
working unmanaged setup, this means that the user chose to manage device
detach/re-attach manually and shouldn't be bothered with a change that's
aimed to managed devices.


Thanks,


DHB



Effectively anyone assigning PFs with a PCIe root port that does not
support ACS would be broken by this series.  That's a significant
portion of vfio users.  Thanks,

Alex



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

[libvirt] [PATCH v2 3/4] util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 53 ++
 1 file changed, 6 insertions(+), 47 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 63680974b8..9485316e05 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -592,6 +592,12 @@ char *virGetUserConfigDirectory(void)
 }
 
 
+char *virGetUserCacheDirectory(void)
+{
+return g_strdup_printf("%s/libvirt", g_get_user_cache_dir());
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -742,29 +748,6 @@ char *virGetUserShell(uid_t uid)
 }
 
 
-static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)
-{
-const char *path = getenv(xdgenvname);
-char *ret = NULL;
-char *home = NULL;
-
-if (path && path[0]) {
-ret = g_strdup_printf("%s/libvirt", path);
-} else {
-home = virGetUserDirectory();
-if (home)
-ret = g_strdup_printf("%s/%s/libvirt", home, xdgdefdir);
-}
-
-VIR_FREE(home);
-return ret;
-}
-
-char *virGetUserCacheDirectory(void)
-{
-return virGetXDGDirectory("XDG_CACHE_HOME", ".cache");
-}
-
 char *virGetUserRuntimeDirectory(void)
 {
 const char *path = getenv("XDG_RUNTIME_DIR");
@@ -1188,21 +1171,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserCacheDirectory(void)
-{
-char *ret;
-if (virGetWin32SpecialFolder(CSIDL_INTERNET_CACHE, &ret) < 0)
-return NULL;
-
-if (!ret) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to determine config directory"));
-return NULL;
-}
-return ret;
-}
-
 char *
 virGetUserRuntimeDirectory(void)
 {
@@ -1228,15 +1196,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserCacheDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserCacheDirectory is not available"));
-
-return NULL;
-}
-
 char *
 virGetUserRuntimeDirectory(void)
 {
-- 
2.23.0

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

[libvirt] [PATCH v2 1/4] util: Rewrite virGetUserDirectory() using g_get_home_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index ed1f696e37..8c255abd7f 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -582,7 +582,7 @@ virGetHostnameQuiet(void)
 char *
 virGetUserDirectory(void)
 {
-return virGetUserDirectoryByUID(geteuid());
+return g_strdup_printf("%s/libvirt", g_get_home_dir());
 }
 
 
-- 
2.23.0

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

[libvirt] [PATCH v2 4/4] util: Rewrite virGetUserRuntimeDirectory() using g_get_user_runtime_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 35 ++-
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 9485316e05..10bf96f193 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -598,6 +598,12 @@ char *virGetUserCacheDirectory(void)
 }
 
 
+char *virGetUserRuntimeDirectory(void)
+{
+return g_strdup_printf("%s/libvirt", g_get_user_runtime_dir());
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -748,20 +754,6 @@ char *virGetUserShell(uid_t uid)
 }
 
 
-char *virGetUserRuntimeDirectory(void)
-{
-const char *path = getenv("XDG_RUNTIME_DIR");
-
-if (!path || !path[0]) {
-return virGetUserCacheDirectory();
-} else {
-char *ret;
-
-ret = g_strdup_printf("%s/libvirt", path);
-return ret;
-}
-}
-
 char *virGetUserName(uid_t uid)
 {
 char *ret;
@@ -1171,12 +1163,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserRuntimeDirectory(void)
-{
-return virGetUserCacheDirectory();
-}
-
 # else /* !HAVE_GETPWUID_R && !WIN32 */
 char *
 virGetUserDirectoryByUID(uid_t uid G_GNUC_UNUSED)
@@ -1195,15 +1181,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 
 return NULL;
 }
-
-char *
-virGetUserRuntimeDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserRuntimeDirectory is not available"));
-
-return NULL;
-}
 # endif /* ! HAVE_GETPWUID_R && ! WIN32 */
 
 char *
-- 
2.23.0

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

[libvirt] [PATCH v2 2/4] util: Rewrite virGetUserConfigDirectory() using g_get_user_config_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 35 ++-
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 8c255abd7f..63680974b8 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -586,6 +586,12 @@ virGetUserDirectory(void)
 }
 
 
+char *virGetUserConfigDirectory(void)
+{
+return g_strdup_printf("%s/libvirt", g_get_user_config_dir());
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -754,11 +760,6 @@ static char *virGetXDGDirectory(const char *xdgenvname, 
const char *xdgdefdir)
 return ret;
 }
 
-char *virGetUserConfigDirectory(void)
-{
-return virGetXDGDirectory("XDG_CONFIG_HOME", ".config");
-}
-
 char *virGetUserCacheDirectory(void)
 {
 return virGetXDGDirectory("XDG_CACHE_HOME", ".cache");
@@ -1187,21 +1188,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserConfigDirectory(void)
-{
-char *ret;
-if (virGetWin32SpecialFolder(CSIDL_LOCAL_APPDATA, &ret) < 0)
-return NULL;
-
-if (!ret) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to determine config directory"));
-return NULL;
-}
-return ret;
-}
-
 char *
 virGetUserCacheDirectory(void)
 {
@@ -1242,15 +1228,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserConfigDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserConfigDirectory is not available"));
-
-return NULL;
-}
-
 char *
 virGetUserCacheDirectory(void)
 {
-- 
2.23.0

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

[libvirt] [PATCH v2 0/4] Rewrite virGetUserDirectory() functions using g_get__dir()

2019-12-17 Thread Fabiano Fidêncio

By rewriting virGetUser*Directory() functions using g_get_*_dir()
functions allows us to drop all the different implementations we
keep, as GLib already takes care of those for us.

Changes since v1:
https://www.redhat.com/archives/libvir-list/2019-December/msg01055.html
- Don't check for the return of g_get_*_dir(), as it cannot be NULL;

Fabiano Fidêncio (4):
  util: Rewrite virGetUserDirectory() using g_get_home_dir()
  util: Rewrite virGetUserConfigDirectory() using
g_get_user_config_dir()
  util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir()
  util: Rewrite virGetUserRuntimeDirectory() using
g_get_user_runtime_dir()

 src/util/virutil.c | 125 +++--
 1 file changed, 19 insertions(+), 106 deletions(-)

-- 
2.23.0

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

Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-17 Thread Alex Williamson

On Tue, 17 Dec 2019 11:25:38 -0500
Laine Stump  wrote:

> On 12/16/19 6:03 PM, Daniel Henrique Barboza wrote:
> >
> >
> > On 12/16/19 7:28 PM, Cole Robinson wrote:  
> >> On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:  
> >>> changes from version 3 [1]:
> >>> - removed last 2 patches that made function 0 of
> >>> PCI multifunction devices mandatory
> >>> - new patch: news.xml update
> >>> - changed 'since' version to 6.0.0 in patch 4
> >>> - unassigned hostdevs are now getting qemu aliases
> >>>
> >>> [1] 
> >>> https://www.redhat.com/archives/libvir-list/2019-November/msg01263.html
> >>>
> >>> Daniel Henrique Barboza (5):
> >>>    Introducing new address type='unassigned' for PCI hostdevs
> >>>    qemu: handle unassigned PCI hostdevs in command line
> >>>    virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices
> >>>    formatdomain.html.in: document 
> >>>    news.xml: add address type='unassigned' entry
> >>>  
> >>
> >> Codewise it looks fine now. But I'm looking more closely at patch #3 and
> >> realizing that it can explicitly reject a previously accepted VM config.
> >> And indeed, now that I give it a test with my GPU passthrough setup, it
> >> is rejecting my previosly working config.
> >>
> >> error: Requested operation is not valid: All devices of the same IOMMU
> >> group 1 of the PCI device :01:00.0 must belong to domain win10
> >>
> >> I've attached the nodedev XML for the three devices with iommuGroup 1.
> >> Only the two nvidia devices are assigned to my VM, but not the PCIe
> >> controller device.
> >>
> >> Is the libvirt heuristic missing something? Or is this acting as 
> >> expected?  
> >
> > You mentioned that you declared 3 devices of IOMMU group 1. Unless the 
> > code in
> > patch 3 has a bug, there are more PCI hostdevs in IOMMU group 1 that 
> > were left
> > out of the domain XML.
> >
> >  
> >>
> >> I didn't quite gather that this is a change to reject previously
> >> accepted configurations, so I will defer to Laine and Alex as to whether
> >> this should be committed.  
> >
> >
> > I mentioned in the commit msg of patch 03 that this would break working
> > configurations that didn't comply with the new 'all devices of the IOMMU
> > group must be included in the domain XML' directive. Perhaps this is 
> > worth
> > mentioning in the 'news' page to warn users about it.  
> 
> 
> No, this shouldn't be a requirement at all. In my mind the purpose of 
> these patches is to make something work (in a safe manner) that failed 
> before, *not* to add new restrictions that break things that already 
> work. (Sorry I wasn't paying more attention to the patches earlier).

+1

> > About breaking existing configurations, there is the possibility of not
> > going forward with patch 03, which is enforcing this rule of declaring 
> > all the
> > IOMMU group. Existing domains will keep working as usual, the option to
> > unassign devices will still be present, but the user will have to deal 
> > with
> > the potential QEMU errors if not all PCI devices were detached from 
> > the host.
> >
> > In this case, the 'unassigned' type will become more of a ON/OFF 
> > switch to
> > add/remove the PCI hostdev from the guest without removing it from the
> > domain XML. It is still useful, but we lose the idea of all the IOMMU
> > devices being described in the domain XML, which is something Laine
> > mentioned it would be desirable in one of the RFCs.  
> 
> 
> I don't actually recall saying that :-). I haven't looked in the list 
> archives, but what I *can* imagine myself saying is that only devices 
> mentioned in the XML should be manipulated in any way by libvirt. So,

+1
 
> for example, you shouldn't unbind device X from its host driver if there 
> is nothing in the XML telling you to do that. But if a device isn't 
> mentioned in the XML, and is already bound to some driver that is 
> acceptable to the VFIO subsystem (e.g. vfio-pci, pci-stub or no driver 
> at all (? is that right Alex?)) then that should not create any problem.

Yes, that's right.

> Doing otherwise would break too many existing configs. (For example, my 
> own assigned-GPU config, which assumes that all the devices are already 
> bound to the proper driver, and uses "managed='no'")

Effectively anyone assigning PFs with a PCIe root port that does not
support ACS would be broken by this series.  That's a significant
portion of vfio users.  Thanks,

Alex

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

Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-17 Thread Laine Stump


On 12/16/19 6:03 PM, Daniel Henrique Barboza wrote:



On 12/16/19 7:28 PM, Cole Robinson wrote:

On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:

changes from version 3 [1]:
- removed last 2 patches that made function 0 of
PCI multifunction devices mandatory
- new patch: news.xml update
- changed 'since' version to 6.0.0 in patch 4
- unassigned hostdevs are now getting qemu aliases

[1] 
https://www.redhat.com/archives/libvir-list/2019-November/msg01263.html


Daniel Henrique Barboza (5):
   Introducing new address type='unassigned' for PCI hostdevs
   qemu: handle unassigned PCI hostdevs in command line
   virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices
   formatdomain.html.in: document 
   news.xml: add address type='unassigned' entry



Codewise it looks fine now. But I'm looking more closely at patch #3 and
realizing that it can explicitly reject a previously accepted VM config.
And indeed, now that I give it a test with my GPU passthrough setup, it
is rejecting my previosly working config.

error: Requested operation is not valid: All devices of the same IOMMU
group 1 of the PCI device :01:00.0 must belong to domain win10

I've attached the nodedev XML for the three devices with iommuGroup 1.
Only the two nvidia devices are assigned to my VM, but not the PCIe
controller device.

Is the libvirt heuristic missing something? Or is this acting as 
expected?


You mentioned that you declared 3 devices of IOMMU group 1. Unless the 
code in
patch 3 has a bug, there are more PCI hostdevs in IOMMU group 1 that 
were left

out of the domain XML.




I didn't quite gather that this is a change to reject previously
accepted configurations, so I will defer to Laine and Alex as to whether
this should be committed.



I mentioned in the commit msg of patch 03 that this would break working
configurations that didn't comply with the new 'all devices of the IOMMU
group must be included in the domain XML' directive. Perhaps this is 
worth

mentioning in the 'news' page to warn users about it.



No, this shouldn't be a requirement at all. In my mind the purpose of 
these patches is to make something work (in a safe manner) that failed 
before, *not* to add new restrictions that break things that already 
work. (Sorry I wasn't paying more attention to the patches earlier).





About breaking existing configurations, there is the possibility of not
going forward with patch 03, which is enforcing this rule of declaring 
all the

IOMMU group. Existing domains will keep working as usual, the option to
unassign devices will still be present, but the user will have to deal 
with
the potential QEMU errors if not all PCI devices were detached from 
the host.


In this case, the 'unassigned' type will become more of a ON/OFF 
switch to

add/remove the PCI hostdev from the guest without removing it from the
domain XML. It is still useful, but we lose the idea of all the IOMMU
devices being described in the domain XML, which is something Laine
mentioned it would be desirable in one of the RFCs.



I don't actually recall saying that :-). I haven't looked in the list 
archives, but what I *can* imagine myself saying is that only devices 
mentioned in the XML should be manipulated in any way by libvirt. So, 
for example, you shouldn't unbind device X from its host driver if there 
is nothing in the XML telling you to do that. But if a device isn't 
mentioned in the XML, and is already bound to some driver that is 
acceptable to the VFIO subsystem (e.g. vfio-pci, pci-stub or no driver 
at all (? is that right Alex?)) then that should not create any problem.


Doing otherwise would break too many existing configs. (For example, my 
own assigned-GPU config, which assumes that all the devices are already 
bound to the proper driver, and uses "managed='no'")




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

Re: [libvirt] [PATCH] ci: Fetch list of available container images dynamically

2019-12-17 Thread Cole Robinson

On 12/11/19 12:56 PM, Andrea Bolognani wrote:
> Any static list of images is destined to become outdated eventually,
> so let's start generating it dynamically instead.
> 
> Unfortunately there doesn't seem to be a straightforward way to get
> Podman/Docker to list all repositories under quay.io/libvirt, so we
> have to resort to searching and filtering manually; and since the
> two tools behave slightly differently in that regard, it's more
> sane to have the logic in a separate shell script than it would be
> to keep it inline in the Makefile with all the annoying escaping
> that would entail.
> 
> Signed-off-by: Andrea Bolognani 

Reviewed-by: Cole Robinson 

But the other ci/ scripts are listed in Makefile.am EXTRA_DIST, so
adjust that before pushing unless it was deliberate

- Cole

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

Re: [libvirt] [PATCH 2/3] qemu: use g_autofree instead of VIR_FREE in qemuMonitorTextCreateSnapshot()

2019-12-17 Thread Cole Robinson

On 12/6/19 4:11 AM, Pavel Mores wrote:
> While at bugfixing, convert the whole function to the new-style memory
> allocation handling.
> 
> Signed-off-by: Pavel Mores 
> ---
>  src/qemu/qemu_monitor_text.c | 18 ++
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 

Reviewed-by: Cole Robinson 

Looks like this patch was missed. Pushed now

- Cole

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

[libvirt] [PATCH 4/4] util: Rewrite virGetUserRuntimeDirectory() using g_get_user_runtime_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 40 +++-
 1 file changed, 11 insertions(+), 29 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 993a959555..62cb3390f8 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -614,6 +614,17 @@ char *virGetUserCacheDirectory(void)
 }
 
 
+char *virGetUserRuntimeDirectory(void)
+{
+const char *runtimedir = g_get_user_runtime_dir();
+
+if (!runtimedir)
+return NULL;
+
+return g_strdup_printf("%s/libvirt", runtimedir);
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -764,20 +775,6 @@ char *virGetUserShell(uid_t uid)
 }
 
 
-char *virGetUserRuntimeDirectory(void)
-{
-const char *path = getenv("XDG_RUNTIME_DIR");
-
-if (!path || !path[0]) {
-return virGetUserCacheDirectory();
-} else {
-char *ret;
-
-ret = g_strdup_printf("%s/libvirt", path);
-return ret;
-}
-}
-
 char *virGetUserName(uid_t uid)
 {
 char *ret;
@@ -1187,12 +1184,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserRuntimeDirectory(void)
-{
-return virGetUserCacheDirectory();
-}
-
 # else /* !HAVE_GETPWUID_R && !WIN32 */
 char *
 virGetUserDirectoryByUID(uid_t uid G_GNUC_UNUSED)
@@ -1211,15 +1202,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 
 return NULL;
 }
-
-char *
-virGetUserRuntimeDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserRuntimeDirectory is not available"));
-
-return NULL;
-}
 # endif /* ! HAVE_GETPWUID_R && ! WIN32 */
 
 char *
-- 
2.23.0

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

[libvirt] [PATCH 0/4] Rewrite virGetUserDirectory() functions using g_get__dir()

2019-12-17 Thread Fabiano Fidêncio

By rewriting virGetUser*Directory() functions using g_get_*_dir()
functions allows us to drop all the different implementations we
keep, as GLib already takes care of those for us.

Fabiano Fidêncio (4):
  util: Rewrite virGetUserDirectory() using g_get_home_dir()
  util: Rewrite virGetUserConfigDirectory() using
g_get_user_config_dir()
  util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir()
  util: Rewrite virGetUserRuntimeDirectory() using
g_get_user_runtime_dir()

 src/util/virutil.c | 146 +
 1 file changed, 40 insertions(+), 106 deletions(-)

-- 
2.23.0

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

[libvirt] [PATCH 2/4] util: Rewrite virGetUserConfigDirectory() using g_get_user_config_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 40 +++-
 1 file changed, 11 insertions(+), 29 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 70a05e3c9b..6fae3e8288 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -592,6 +592,17 @@ virGetUserDirectory(void)
 }
 
 
+char *virGetUserConfigDirectory(void)
+{
+const char *configdir = g_get_user_config_dir();
+
+if (!configdir)
+return NULL;
+
+return g_strdup_printf("%s/libvirt", configdir);
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -760,11 +771,6 @@ static char *virGetXDGDirectory(const char *xdgenvname, 
const char *xdgdefdir)
 return ret;
 }
 
-char *virGetUserConfigDirectory(void)
-{
-return virGetXDGDirectory("XDG_CONFIG_HOME", ".config");
-}
-
 char *virGetUserCacheDirectory(void)
 {
 return virGetXDGDirectory("XDG_CACHE_HOME", ".cache");
@@ -1193,21 +1199,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserConfigDirectory(void)
-{
-char *ret;
-if (virGetWin32SpecialFolder(CSIDL_LOCAL_APPDATA, &ret) < 0)
-return NULL;
-
-if (!ret) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to determine config directory"));
-return NULL;
-}
-return ret;
-}
-
 char *
 virGetUserCacheDirectory(void)
 {
@@ -1248,15 +1239,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserConfigDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserConfigDirectory is not available"));
-
-return NULL;
-}
-
 char *
 virGetUserCacheDirectory(void)
 {
-- 
2.23.0

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

[libvirt] [PATCH 3/4] util: Rewrite virGetUserCacheDirectory() using g_get_user_cache_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 58 +-
 1 file changed, 11 insertions(+), 47 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 6fae3e8288..993a959555 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -603,6 +603,17 @@ char *virGetUserConfigDirectory(void)
 }
 
 
+char *virGetUserCacheDirectory(void)
+{
+const char *cachedir = g_get_user_cache_dir();
+
+if (!cachedir)
+return NULL;
+
+return g_strdup_printf("%s/libvirt", cachedir);
+}
+
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error if not instructed otherwise via @quiet,
@@ -753,29 +764,6 @@ char *virGetUserShell(uid_t uid)
 }
 
 
-static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)
-{
-const char *path = getenv(xdgenvname);
-char *ret = NULL;
-char *home = NULL;
-
-if (path && path[0]) {
-ret = g_strdup_printf("%s/libvirt", path);
-} else {
-home = virGetUserDirectory();
-if (home)
-ret = g_strdup_printf("%s/%s/libvirt", home, xdgdefdir);
-}
-
-VIR_FREE(home);
-return ret;
-}
-
-char *virGetUserCacheDirectory(void)
-{
-return virGetXDGDirectory("XDG_CACHE_HOME", ".cache");
-}
-
 char *virGetUserRuntimeDirectory(void)
 {
 const char *path = getenv("XDG_RUNTIME_DIR");
@@ -1199,21 +1187,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserCacheDirectory(void)
-{
-char *ret;
-if (virGetWin32SpecialFolder(CSIDL_INTERNET_CACHE, &ret) < 0)
-return NULL;
-
-if (!ret) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Unable to determine config directory"));
-return NULL;
-}
-return ret;
-}
-
 char *
 virGetUserRuntimeDirectory(void)
 {
@@ -1239,15 +1212,6 @@ virGetUserShell(uid_t uid G_GNUC_UNUSED)
 return NULL;
 }
 
-char *
-virGetUserCacheDirectory(void)
-{
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("virGetUserCacheDirectory is not available"));
-
-return NULL;
-}
-
 char *
 virGetUserRuntimeDirectory(void)
 {
-- 
2.23.0

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

[libvirt] [PATCH 1/4] util: Rewrite virGetUserDirectory() using g_get_home_dir()

2019-12-17 Thread Fabiano Fidêncio

Signed-off-by: Fabiano Fidêncio 
---
 src/util/virutil.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index ed1f696e37..70a05e3c9b 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -582,7 +582,13 @@ virGetHostnameQuiet(void)
 char *
 virGetUserDirectory(void)
 {
-return virGetUserDirectoryByUID(geteuid());
+const char *homedir;
+
+homedir = g_get_home_dir();
+if (!homedir)
+return NULL;
+
+return g_strdup_printf("%s/libvirt", homedir);
 }
 
 
-- 
2.23.0

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

Re: [libvirt] [PATCH] tests: securityselinuxlabel: Add QEMU_CAPS_VNC to fake qemuCaps

2019-12-17 Thread Cole Robinson

On 12/17/19 4:04 AM, Peter Krempa wrote:
> In commit 45270337f057f26ce484f6e forgot to make sure that tests pass.
> Add the missing capability to fix the test.
> 

Thanks for the fixing this. FWIW I tested every commit individually,
multiple times over as I was poking at this series over a few days. At
one point I was hitting securityselinuxlabeltest failures, but when I
would run the test directly it succeeded. Then I rebased to master and
the test failure went away and I couldn't reproduce. I pushed the series
to another machine and tests passed there too.

There must be something that makes the test flakey, or ordering
dependent maybe? Daniel obviously was running at least some portion of
the test suite, considering most patches in the series had to adjust the
test suite to match.

- Cole

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

Re: [libvirt] [PATCH 0/9] Cleanup virConnectPtr usage

2019-12-17 Thread Cole Robinson

On 12/4/19 4:55 AM, Michal Privoznik wrote:
> I've noticed this problem when reviewing a patch on the list [1].
> 
> Long story short, dom->conn is not guaranteed to have all driver
> pointers set (consider split daemons). I haven't identified any
> other violation than what I'm fixing here. But something might have
> slipped through my git grep.
> 
> 1: https://www.redhat.com/archives/libvir-list/2019-December/msg00120.html
> 
> Michal Prívozník (9):
>   qemu_driver: Push qemuDomainInterfaceAddresses() a few lines down
>   qemu: Don't use dom->conn to lookup virNetwork
>   qemuGetDHCPInterfaces: Move some variables inside the loop
>   qemuGetDHCPInterfaces: Switch to GLib
>   libxl: Don't use dom->conn to lookup virNetwork
>   libxlGetDHCPInterfaces: Move some variables inside the loop
>   libxlGetDHCPInterfaces: Switch to GLib
>   lxc: Cleanup virConnectPtr usage
>   get_nonnull_domain: Drop useless comment
> 
>  src/libxl/libxl_driver.c|  71 --
>  src/lxc/lxc_driver.c|  12 +-
>  src/lxc/lxc_process.c   |  40 +++---
>  src/lxc/lxc_process.h   |   2 +-
>  src/qemu/qemu_driver.c  | 193 
>  src/remote/remote_daemon_dispatch.c |   3 -
>  6 files changed, 139 insertions(+), 182 deletions(-)
> 

Reviewed-by: Cole Robinson 

- Cole

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

Re: [libvirt] [PATCH] cpu_map/x86: Add support for BFLOAT16 data type

2019-12-17 Thread Ján Tomko


On Fri, Dec 13, 2019 at 05:27:13PM +0100, Jiri Denemark wrote:

Introduced in QEMU by commit v4.1.0-266-g80db491da4.


Please delete the trailing dot, to make selecting the commit ID easier.



Signed-off-by: Jiri Denemark 
---
src/cpu_map/x86_features.xml | 4 
1 file 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] virsh migrate: Require --tls for --tls-destination

2019-12-17 Thread Peter Krempa

On Tue, Dec 17, 2019 at 15:02:24 +0100, Jiri Denemark wrote:
> On Tue, Dec 17, 2019 at 14:46:24 +0100, Peter Krempa wrote:
> > On Tue, Dec 17, 2019 at 14:34:12 +0100, Jiri Denemark wrote:
> > > --tls-destination would be just ignored unless --tls is not specified,
> > > which is correct, but let's provide a bit of a guidance is a user
> > > forgets to add --tls.
> > > 
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1784345
> > > 
> > > Signed-off-by: Jiri Denemark 
> > > ---
> > >  tools/virsh-domain.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > > index 56137bdd74..e7e92ee60d 100644
> > > --- a/tools/virsh-domain.c
> > > +++ b/tools/virsh-domain.c
> > > @@ -10966,6 +10966,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
> > >  VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy");
> > >  VSH_REQUIRE_OPTION("timeout-postcopy", "postcopy");
> > >  VSH_REQUIRE_OPTION("persistent-xml", "persistent");
> > > +VSH_REQUIRE_OPTION("tls-destination", "tls");
> > 
> > This fixes just virsh users. What about direct API users? Shouldn't the
> > bug be fixed at API level too?
> 
> We currently don't do such checks at API level. Migration parameters
> which depend on a flag are checked only if the flag is set, otherwise
> they are just ignored. This is probably not optimal, but it's not
> incorrect :-)

While I think that what you wrote is a ridiculously weak excuse, I see
that we do such a weird thing in other instances as well.

Note that the documentation for the parameter does't mention the above
either.

ACK if you amend the commit message stating that this is a virsh-only
hack done on purpose, so that it doesn't look like I forgot to mention
the API-level check during review.

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

Re: [libvirt] [glib PATCH 0/2] po: change over to use Weblate instead of Zanata

2019-12-17 Thread Michal Prívozník

On 12/16/19 1:40 PM, Daniel P. Berrangé wrote:
> Zanata is dead upstream, and Fedora is replacing it with Weblate
> 
>   https://fedoraproject.org/wiki/L10N_Move_to_Weblate
> 
> Libvirt has outsourced its translations to the Fedora translation
> team, so we must follow this move.
> 
> Daniel P. Berrangé (2):
>   po: delete empty translation files
>   po: change update rules to use weblate instead of zanata
> 
>  po/Makefile.am  | 32 +++-
>  po/af.mini.po   | 20 
>  po/am.mini.po   | 20 
>  po/anp.mini.po  | 19 ---
>  po/ar.mini.po   | 21 -
>  po/as.mini.po   | 20 
>  po/ast.mini.po  | 20 
>  po/bal.mini.po  | 20 
>  po/be.mini.po   | 21 -
>  po/bg.mini.po   | 20 
>  po/bn.mini.po   | 20 
>  po/bn_IN.mini.po| 20 
>  po/bo.mini.po   | 20 
>  po/br.mini.po   | 20 
>  po/brx.mini.po  | 20 
>  po/bs.mini.po   | 21 -
>  po/cy.mini.po   | 21 -
>  po/da.mini.po   | 20 
>  po/de.mini.po   | 20 
>  po/de_CH.mini.po| 20 
>  po/el.mini.po   | 20 
>  po/eo.mini.po   | 20 
>  po/et.mini.po   | 20 
>  po/eu.mini.po   | 20 
>  po/fa.mini.po   | 20 
>  po/gl.mini.po   | 20 
>  po/gu.mini.po   | 20 
>  po/he.mini.po   | 20 
>  po/hr.mini.po   | 21 -
>  po/hu.mini.po   | 20 
>  po/ia.mini.po   | 20 
>  po/id.mini.po   | 20 
>  po/ilo.mini.po  | 20 
>  po/is.mini.po   | 20 
>  po/it.mini.po   | 20 
>  po/ka.mini.po   | 20 
>  po/kk.mini.po   | 20 
>  po/km.mini.po   | 20 
>  po/kn.mini.po   | 20 
>  po/ko.mini.po   | 20 
>  po/kw.mini.po   | 20 
>  po/k...@kkcor.mini.po | 20 
>  po/k...@uccor.mini.po | 20 
>  po/kw_GB.mini.po| 20 
>  po/ky.mini.po   | 20 
>  po/lt.mini.po   | 21 -
>  po/lv.mini.po   | 21 -
>  po/mai.mini.po  | 20 
>  po/mk.mini.po   | 20 
>  po/ml.mini.po   | 20 
>  po/mn.mini.po   | 20 
>  po/mr.mini.po   | 20 
>  po/ms.mini.po   | 20 
>  po/nb.mini.po   | 20 
>  po/nds.mini.po  | 20 
>  po/ne.mini.po   | 20 
>  po/nl.mini.po   | 20 
>  po/nn.mini.po   | 20 
>  po/nso.mini.po  | 20 
>  po/or.mini.po   | 20 
>  po/pa.mini.po   | 20 
>  po/pt.mini.po   | 20 
>  po/ro.mini.po   | 21 -
>  po/si.mini.po   | 20 
>  po/sk.mini.po   | 20 
>  po/sl.mini.po   | 21 -
>  po/sq.mini.po   | 20 
>  po/sr.mini.po   | 21 -
>  po/s...@latin.mini.po | 21 -
>  po/sv.mini.po   | 20 
>  po/ta.mini.po   | 20 
>  po/te.mini.po   | 20 
>  po/tg.mini.po   | 20 
>  po/th.mini.po   | 20 
>  po/tr.mini.po   | 20 
>  po/tw.mini.po   | 19 ---
>  po/ur.mini.po   | 20 
>  po/vi.mini.po   | 20 
>  po/wba.mini.po  | 19 ---
>  po/yo.mini.po   | 19 ---
>  po/zanata.xml   |  7 ---
>  po/zh_CN.mini.po| 20 
>  po/zh_HK.mini.po| 20 
>  po/zh_TW.mini.po| 20 
>  po/zu.mini.po   | 20 
>  85 files changed, 19 insertions(+), 1687 deletions(-)
>  delete mode 100644 po/af.mini.po
>  delete mode 100644 po/am.mini.po
>  delete mode 100644 po/anp.mini.po
>  delete mode 100644 po/ar.mini.po
>  delete mode 100644 po/as.mini.po
>  delete mode 100644 po/ast.mini.po
>  delete mode 100644 po/bal.mini.po
>  delete mode 100644 po/be.mini.po
>  delete mode 100644 po/bg.mini.po
>  delete mode 100644 po/

Re: [libvirt] [glib PATCH 2/2] po: change update rules to use weblate instead of zanata

2019-12-17 Thread Michal Prívozník

On 12/16/19 1:40 PM, Daniel P. Berrangé wrote:
> The Zanata project is dead and so Fedora is transitioning over to using
> Weblate for managing translations. With Weblate, languages are only
> created on the server when the first translation is started, so we must
> check to see if any new languages exist during download, so that they
> can be added to the local languages list.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  po/Makefile.am | 20 +---
>  po/zanata.xml  |  7 ---
>  2 files changed, 17 insertions(+), 10 deletions(-)
>  delete mode 100644 po/zanata.xml

Don't forget to update po/README.md as it documents zanata too.

Michal

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

Re: [libvirt] [PATCH] virsh migrate: Require --tls for --tls-destination

2019-12-17 Thread Jiri Denemark

On Tue, Dec 17, 2019 at 14:46:24 +0100, Peter Krempa wrote:
> On Tue, Dec 17, 2019 at 14:34:12 +0100, Jiri Denemark wrote:
> > --tls-destination would be just ignored unless --tls is not specified,
> > which is correct, but let's provide a bit of a guidance is a user
> > forgets to add --tls.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1784345
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  tools/virsh-domain.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 56137bdd74..e7e92ee60d 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -10966,6 +10966,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
> >  VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy");
> >  VSH_REQUIRE_OPTION("timeout-postcopy", "postcopy");
> >  VSH_REQUIRE_OPTION("persistent-xml", "persistent");
> > +VSH_REQUIRE_OPTION("tls-destination", "tls");
> 
> This fixes just virsh users. What about direct API users? Shouldn't the
> bug be fixed at API level too?

We currently don't do such checks at API level. Migration parameters
which depend on a flag are checked only if the flag is set, otherwise
they are just ignored. This is probably not optimal, but it's not
incorrect :-)

Jirka

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

Re: [libvirt] [PATCH] virsh migrate: Require --tls for --tls-destination

2019-12-17 Thread Peter Krempa

On Tue, Dec 17, 2019 at 14:34:12 +0100, Jiri Denemark wrote:
> --tls-destination would be just ignored unless --tls is not specified,
> which is correct, but let's provide a bit of a guidance is a user
> forgets to add --tls.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1784345
> 
> Signed-off-by: Jiri Denemark 
> ---
>  tools/virsh-domain.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 56137bdd74..e7e92ee60d 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -10966,6 +10966,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
>  VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy");
>  VSH_REQUIRE_OPTION("timeout-postcopy", "postcopy");
>  VSH_REQUIRE_OPTION("persistent-xml", "persistent");
> +VSH_REQUIRE_OPTION("tls-destination", "tls");

This fixes just virsh users. What about direct API users? Shouldn't the
bug be fixed at API level too?

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

[libvirt] [PATCH] virsh migrate: Require --tls for --tls-destination

2019-12-17 Thread Jiri Denemark

--tls-destination would be just ignored unless --tls is not specified,
which is correct, but let's provide a bit of a guidance is a user
forgets to add --tls.

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

Signed-off-by: Jiri Denemark 
---
 tools/virsh-domain.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 56137bdd74..e7e92ee60d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10966,6 +10966,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
 VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy");
 VSH_REQUIRE_OPTION("timeout-postcopy", "postcopy");
 VSH_REQUIRE_OPTION("persistent-xml", "persistent");
+VSH_REQUIRE_OPTION("tls-destination", "tls");
 
 if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
 return false;
-- 
2.24.1

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

[libvirt] [PATCH] remote_daemon: Log host boot time

2019-12-17 Thread Michal Privoznik

This is not strictly needed, but it makes sure we initialize the
@bootTime global variable. Thing is, in order to validate XATTRs
and prune those set in some previous runs of the host, a
timestamp is recorded in XATTRs. The host boot time was unique
enough so it was chosen as the timestamp value. And to avoid
querying and parsing /proc/uptime every time, the query function
does that only once and stores the boot time in a global
variable. However, the only time the query function is called is
in a child process that does lock files and changes seclabels. So
effectively, we are doing exactly what we wanted to prevent from
happening.

The fix is simple, call the query function whilst the daemon is
initializing.

Signed-off-by: Michal Privoznik 
---

Since this will be used by security driver only, I was tempted to put
this somewhere under src/security/, but especially because of split
daemon I didn't. Placing this into remote_daemon.c makes sure all
sub-daemons will have the variable initialized and won't suffer the
problem.

 src/remote/remote_daemon.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index b400b1dd10..5debb6ce97 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -57,6 +57,7 @@
 #include "virgettext.h"
 #include "util/virnetdevopenvswitch.h"
 #include "virsystemd.h"
+#include "virhostuptime.h"
 
 #include "driver.h"
 
@@ -1020,6 +1021,7 @@ int main(int argc, char **argv) {
 bool implicit_conf = false;
 char *run_dir = NULL;
 mode_t old_umask;
+unsigned long long bootTime;
 
 struct option opts[] = {
 { "verbose", no_argument, &verbose, 'v'},
@@ -1151,6 +1153,12 @@ int main(int argc, char **argv) {
 exit(EXIT_FAILURE);
 }
 
+if (virHostGetBootTime(&bootTime) < 0) {
+VIR_DEBUG("Unable to get host boot time");
+} else {
+VIR_DEBUG("host boot time: %lld", bootTime);
+}
+
 daemonSetupNetDevOpenvswitch(config);
 
 if (daemonSetupAccessManager(config) < 0) {
-- 
2.24.1

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

Re: [libvirt] [PATCH v3 00/30] Introduce NVMe support

2019-12-17 Thread Michal Prívozník

On 12/2/19 3:26 PM, Michal Privoznik wrote:
>

This is pushed now. Thanks Cole for review!

Michal

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

Re: [libvirt] [PATCH 2/2] build: relax the relaxed stack frame limit further

2019-12-17 Thread Daniel Henrique Barboza




On 12/14/19 6:49 PM, Ján Tomko wrote:

Pick 256k as the limit.

While -Wno-frame-larger-than would make more sense for usage
in our test suite, the -Wno version seems to have no effect
if -Wframe-larger-than was already specified.

Use an (un)reasonably large value instead.

Fixes the build with clang:
../../tests/cputest.c:964:1: error: stack frame size of 33176 bytes
in function 'mymain' [-Werror,-Wframe-larger-than=]
mymain(void)
^
1 error generated.

Signed-off-by: Ján Tomko 
---



Reviewed-by: Daniel Henrique Barboza 


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

Re: [libvirt] [PATCH 1/2] build: warn on a large frame by default

2019-12-17 Thread Daniel Henrique Barboza




On 12/14/19 6:49 PM, Ján Tomko wrote:

My commit e73889b6311f5b43d859caa4bae84bfdb299967a
split the -Wframe-larger-than warning setting into
two different variables - STRICT_FRAME_LIMIT_CFLAGS
for the library code and RELAXED_FRAME_LIMIT_CFLAGS
which was needed for tests.

Use the strict limit by default and specify the warning
flag twice for the parts that require a larger stack
frame, relying on the fact that the compiler will pick
up the latter value.

Signed-off-by: Ján Tomko 
---


Reviewed-by: Daniel Henrique Barboza 


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

Re: [libvirt] [PATCH] virtio-blk: deprecate SCSI passthrough

2019-12-17 Thread Stefan Hajnoczi

On Fri, Dec 13, 2019 at 02:46:26PM +, Stefan Hajnoczi wrote:
> The Linux virtio_blk.ko guest driver is removing legacy SCSI passthrough
> support.  Deprecate this feature in QEMU too.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  qemu-deprecated.texi | 11 +++
>  1 file changed, 11 insertions(+)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


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

[libvirt] [PATCH] tests: securityselinuxlabel: Add QEMU_CAPS_VNC to fake qemuCaps

2019-12-17 Thread Peter Krempa

In commit 45270337f057f26ce484f6e forgot to make sure that tests pass.
Add the missing capability to fix the test.

Signed-off-by: Peter Krempa 
---

Pushed.

 tests/securityselinuxlabeltest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index 192f2dc84f..3040a36693 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -351,6 +351,7 @@ mymain(void)
 return EXIT_FAILURE;

 virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC);

 if (qemuTestCapsCacheInsert(driver.qemuCapsCache, qemuCaps) < 0)
 return EXIT_FAILURE;
-- 
2.23.0

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

95 matches

Mail list logo