Re: [PATCH] qemu_cgroup.c: use VIR_AUTOSTRINGLIST, g_autofree and g_autoptr

[Daniel Henrique Barboza]

On 3/31/20 12:44 PM, Seeteena Thoufeek wrote:

Signed-off-by: Seeteena Thoufeek 
---


Reviewed-by: Daniel Henrique Barboza 

Re: [GSoC][RFC] Project Proposal: "Introducing Job control to the storage driver"

[Prathamesh Chavan]

On Tue, Mar 31, 2020 at 9:34 PM Michal Prívozník  wrote:
>
> On 31. 3. 2020 17:35, Prathamesh Chavan wrote:
> > On Tue, Mar 31, 2020 at 3:20 PM Michal Prívozník  
> > wrote:
> >>
> >> On 31. 3. 2020 2:27, Prathamesh Chavan wrote:
> >>> GSoC Proposal - 2020
> >>> 
> >>
> >> Looks good. It's visible that you've already participated in GSoC
> >> previously. Just don't forget to make it final. The deadline is in ~8 
> >> hours.
> >>
> >> Michal
> >>
> >
> > Thanks for the feedback.
> > I've made the final submission on the GSoC website. The updated document
> > is available here [1].
> >
> > I wanted to post certain questions regarding the project. Will it be okay if
> > I cc'd you and Pavel to the emails and post them on the mailing list?
> > Or should I be asking them on the IRC-channel?
> >
> > While going through the archives I wasn't able to fin much of the GSoC 
> > project
> > discussions on the mailing list. Hence, felt the need to ask this question.
>
> Yes, discussing this on the list is fine. Usually, students write a
> private e-mail to mentors, but since some ideas don't have mentors
> assigned (yet), let's discuss on the list. And feel free to CC me.
>

Thanks for informing me.
For now, I'll be using the mailing-list itself for the discussion and
will cc you and Pavel.

Thanks,
Prathamesh Chavan

[PATCH v3 09/10] qemu: Make memory path generation embed driver aware

[Michal Privoznik]

So far, libvirt generates the following path for memory:

  $memoryBackingDir/$id-$shortName/ram-nodeN

where $memoryBackingDir is the path where QEMU mmaps() memory for
the guest (e.g. /var/lib/libvirt/qemu/ram), $id is domain ID
and $shortName is shortened version of domain name. So for
instance, the generated path may look something like this:

  /var/lib/libvirt/qemu/ram/1-QEMUGuest/ram-node0

While in case of embed driver the following path would be
generated by default:

  $root/lib/qemu/ram/1-QEMUGuest/ram-node0

which is not clashing with other embed drivers, we allow users to
override the default and have all embed drivers use the same
prefix. This can create clashing paths. Fortunately, we can reuse
the approach for machined name generation
(v6.1.0-178-gc9bd08ee35) and include part of hash of the root in
the generated path.

Note, the important change is in qemuGetMemoryBackingBasePath().
The rest is needed to pass driver around.

Signed-off-by: Michal Privoznik 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_command.c | 15 +++
 src/qemu/qemu_conf.c| 21 ++---
 src/qemu/qemu_conf.h|  8 
 src/qemu/qemu_process.c |  5 ++---
 4 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 689796a92b..7ffe59643b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3472,7 +3472,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
 } else {
 /* We can have both pagesize and mem source. If that's the case,
  * prefer hugepages as those are more specific. */
-if (qemuGetMemoryBackingPath(def, cfg, mem->info.alias, ) 
< 0)
+if (qemuGetMemoryBackingPath(priv->driver, def, mem->info.alias, 
) < 0)
 return -1;
 }
 
@@ -7233,11 +7233,11 @@ qemuBuildSmpCommandLine(virCommandPtr cmd,
 
 
 static int
-qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
-const virDomainDef *def,
+qemuBuildMemPathStr(const virDomainDef *def,
 virCommandPtr cmd,
 qemuDomainObjPrivatePtr priv)
 {
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
 const long system_page_size = virGetSystemPageSizeKB();
 g_autofree char *mem_path = NULL;
 
@@ -7254,7 +7254,7 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
 if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, _path) 
< 0)
 return -1;
 } else if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
-if (qemuGetMemoryBackingPath(def, cfg, "ram", _path) < 0)
+if (qemuGetMemoryBackingPath(priv->driver, def, "ram", _path) < 0)
 return -1;
 } else {
 return 0;
@@ -7273,7 +7273,6 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
 
 static int
 qemuBuildMemCommandLine(virCommandPtr cmd,
-virQEMUDriverConfigPtr cfg,
 const virDomainDef *def,
 virQEMUCapsPtr qemuCaps,
 qemuDomainObjPrivatePtr priv)
@@ -7305,7 +7304,7 @@ qemuBuildMemCommandLine(virCommandPtr cmd,
  * the hugepages and no numa node is specified.
  */
 if (!virDomainNumaGetNodeCount(def->numa) &&
-qemuBuildMemPathStr(cfg, def, cmd, priv) < 0)
+qemuBuildMemPathStr(def, cmd, priv) < 0)
 return -1;
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OVERCOMMIT)) {
@@ -7386,7 +7385,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 }
 
 if (!needBackend &&
-qemuBuildMemPathStr(cfg, def, cmd, priv) < 0)
+qemuBuildMemPathStr(def, cmd, priv) < 0)
 goto cleanup;
 
 for (i = 0; i < ncells; i++) {
@@ -9879,7 +9878,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
 if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) < 0)
 return NULL;
 
-if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps, priv) < 0)
+if (qemuBuildMemCommandLine(cmd, def, qemuCaps, priv) < 0)
 return NULL;
 
 if (qemuBuildSmpCommandLine(cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 713542f8cd..c59824006c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1965,16 +1965,23 @@ qemuGetDomainHupageMemPath(virQEMUDriverPtr driver,
 
 
 int
-qemuGetMemoryBackingDomainPath(const virDomainDef *def,
-   virQEMUDriverConfigPtr cfg,
+qemuGetMemoryBackingDomainPath(virQEMUDriverPtr driver,
+   const virDomainDef *def,
char **path)
 {
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+const char *root = driver->embeddedRoot;
 g_autofree char *shortName = NULL;
 
 if (!(shortName = virDomainDefGetShortName(def)))
 return -1;
 
-*path = g_strdup_printf("%s/%s", cfg->memoryBackingDir, 

[PATCH v3 10/10] qemu: Make auto dump path generation embed driver aware

[Michal Privoznik]

So far, libvirt generates the following path for automatic dumps:

  $autoDumpPath/$id-$shortName-$timestamp

where $autoDumpPath is where libvirt stores dumps of guests (e.g.
/var/lib/libvirt/qemu/dump), $id is domain ID and $shortName is
shortened version of domain name. So for instance, the generated
path may look something like this:

  /var/lib/libvirt/qemu/dump/1-QEMUGuest-2020-03-25-10:40:50

While in case of embed driver the following path would be
generated by default:

  $root/lib/libvirt/qemu/dump/1-QEMUGuest-2020-03-25-10:40:50

which is not clashing with other embed drivers, we allow users to
override the default and have all embed drivers use the same
prefix. This can create clashing paths. Fortunately, we can reuse
the approach for machined name generation
(v6.1.0-178-gc9bd08ee35) and include part of hash of the root in
the generated path.

Signed-off-by: Michal Privoznik 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 33f177adbd..1746c85be4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4104,6 +4104,7 @@ static char *
 getAutoDumpPath(virQEMUDriverPtr driver,
 virDomainObjPtr vm)
 {
+const char *root = driver->embeddedRoot;
 g_autofree char *domname = virDomainDefGetShortName(vm->def);
 g_autoptr(GDateTime) now = g_date_time_new_now_local();
 g_autofree char *nowstr = NULL;
@@ -4116,6 +4117,11 @@ getAutoDumpPath(virQEMUDriverPtr driver,
 
 nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S");
 
+if (root && !STRPREFIX(cfg->autoDumpPath, root)) {
+g_autofree char * hash = 
virDomainDriverGenerateRootHash(QEMU_DRIVER_NAME, root);
+return g_strdup_printf("%s/%s-%s-%s", cfg->autoDumpPath, hash, 
domname, nowstr);
+}
+
 return g_strdup_printf("%s/%s-%s", cfg->autoDumpPath, domname, nowstr);
 }
 
-- 
2.24.1

[PATCH v3 05/10] virDomainDriverGenerateMachineName: Factor out embed path hashing

[Michal Privoznik]

The code that generates "qemu-embed-$hash" is going to be useful
in more places. Separate it out into a function.

Signed-off-by: Michal Privoznik 
Reviewed-by: Andrea Bolognani 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 src/hypervisor/domain_driver.c | 42 ++
 src/hypervisor/domain_driver.h |  4 
 src/libvirt_private.syms   |  1 +
 3 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index 7bf0fb3f98..31821fc712 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -28,6 +28,22 @@
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
+char *
+virDomainDriverGenerateRootHash(const char *drivername,
+const char *root)
+{
+g_autofree char *hash = NULL;
+
+if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, root, ) < 0)
+return NULL;
+
+/* When two embed drivers start two domains with the same @name and @id
+ * we would generate a non-unique name. Include parts of hashed @root
+ * which guarantees uniqueness. The first 8 characters of SHA256 ought
+ * to be enough for anybody. */
+return g_strdup_printf("%s-embed-%.8s", drivername, hash);
+}
+
 
 #define HOSTNAME_CHARS \
 "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-"
@@ -72,26 +88,24 @@ virDomainDriverGenerateMachineName(const char *drivername,
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-virBufferAsprintf(, "%s-", drivername);
-
 if (root) {
 g_autofree char *hash = NULL;
 
-/* When two embed drivers start two domains with the same @name and @id
- * we would generate a non-unique name. Include parts of hashed @root
- * which guarantees uniqueness. The first 8 characters of SHA256 ought
- * to be enough for anybody. */
-if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, root, ) < 0)
+if (!(hash = virDomainDriverGenerateRootHash(drivername, root)))
 return NULL;
 
-virBufferAsprintf(, "embed-%.8s-", hash);
-} else if (!privileged) {
-g_autofree char *username = NULL;
-if (!(username = virGetUserName(geteuid( {
-virBufferFreeAndReset();
-return NULL;
+virBufferAsprintf(, "%s-", hash);
+} else {
+virBufferAsprintf(, "%s-", drivername);
+if (!privileged) {
+
+g_autofree char *username = NULL;
+if (!(username = virGetUserName(geteuid( {
+virBufferFreeAndReset();
+return NULL;
+}
+virBufferAsprintf(, "%s-", username);
 }
-virBufferAsprintf(, "%s-", username);
 }
 
 virBufferAsprintf(, "%d-", id);
diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h
index c52e37f038..b66ae2d421 100644
--- a/src/hypervisor/domain_driver.h
+++ b/src/hypervisor/domain_driver.h
@@ -22,6 +22,10 @@
 
 #include "domain_conf.h"
 
+char *
+virDomainDriverGenerateRootHash(const char *drivername,
+const char *root);
+
 char *
 virDomainDriverGenerateMachineName(const char *drivername,
const char *root,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b02b6380ed..ec367653d5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1403,6 +1403,7 @@ virDomainCgroupSetupMemtune;
 
 # hypervisor/domain_driver.h
 virDomainDriverGenerateMachineName;
+virDomainDriverGenerateRootHash;
 virDomainDriverMergeBlkioDevice;
 virDomainDriverParseBlkioDeviceStr;
 virDomainDriverSetupPersistentDefBlkioParams;
-- 
2.24.1

[PATCH v3 00/10] Another round of qemu:///embed fixes

[Michal Privoznik]

v3 of:

https://www.redhat.com/archives/libvir-list/2020-March/msg01112.html

diff to v2:
- Patch 2/10 is new; after discussion to v2 we can access
  driver->privileged and driver->embeddedRoot directly
- Patch 3/10 is slightly reworked; it still drops unnecessary layers of
  nesting in the default case (/var/lib/libvirt/qemu/ram) but it doesn't
  do that if user provides a path via qemu.conf

Michal Prívozník (10):
  tests: Fix virQEMUDriverConfigNew() calling with respect to @root
  qemu: Drop virQEMUDriverIsPrivileged()
  qemu: Drop two layers of nesting of memoryBackingDir
  conf: Move virDomainGenerateMachineName to hypervisor/
  virDomainDriverGenerateMachineName: Factor out embed path hashing
  qemuDomainGetMachineName: Access embeddedRoot from driver rather than
cfg
  Revert "qemu_conf: Track embed root dir"
  qemu: Make hugepages path generation embed driver aware
  qemu: Make memory path generation embed driver aware
  qemu: Make auto dump path generation embed driver aware

 src/conf/domain_conf.c| 72 ---
 src/conf/domain_conf.h|  7 --
 src/hypervisor/domain_driver.c| 88 +++
 src/hypervisor/domain_driver.h| 11 +++
 src/libvirt_private.syms  |  3 +-
 src/lxc/lxc_domain.c  |  3 +-
 src/qemu/qemu_cgroup.c|  4 +-
 src/qemu/qemu_command.c   | 23 +++--
 src/qemu/qemu_conf.c  | 76 +---
 src/qemu/qemu_conf.h  | 23 +++--
 src/qemu/qemu_domain.c| 19 ++--
 src/qemu/qemu_driver.c| 42 -
 src/qemu/qemu_interface.c |  6 +-
 src/qemu/qemu_process.c   |  7 +-
 tests/domaincapstest.c|  2 +-
 .../qemuxml2argvdata/cpu-numa-memshared.args  |  8 +-
 .../fd-memory-no-numa-topology.args   |  2 +-
 .../fd-memory-numa-topology.args  |  4 +-
 .../fd-memory-numa-topology2.args |  8 +-
 .../fd-memory-numa-topology3.args | 12 +--
 .../hugepages-memaccess2.args | 12 +--
 .../qemuxml2argvdata/pages-dimm-discard.args  |  4 +-
 ...vhost-user-fs-fd-memory.x86_64-latest.args |  2 +-
 tests/testutilsqemu.c |  2 +-
 tests/virsystemdtest.c|  5 +-
 25 files changed, 235 insertions(+), 210 deletions(-)

-- 
2.24.1

[PATCH v3 06/10] qemuDomainGetMachineName: Access embeddedRoot from driver rather than cfg

[Michal Privoznik]

The cfg->root is going away, therefore get the info right from
the driver structure.

Signed-off-by: Michal Privoznik 
Reviewed-by: Andrea Bolognani 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 25ee46fe34..cba11b9e1e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -16535,7 +16535,6 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverPtr driver = priv->driver;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 char *ret = NULL;
 
 if (vm->pid > 0) {
@@ -16545,7 +16544,8 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
 }
 
 if (!ret)
-ret = virDomainDriverGenerateMachineName("qemu", cfg->root,
+ret = virDomainDriverGenerateMachineName("qemu",
+ driver->embeddedRoot,
  vm->def->id, vm->def->name,
  driver->privileged);
 
-- 
2.24.1

[PATCH v3 08/10] qemu: Make hugepages path generation embed driver aware

[Michal Privoznik]

So far, libvirt generates the following path for hugepages:

  $mnt/libvirt/qemu/$id-$shortName

where $mnt is the mount point of hugetlbfs corresponding to
hugepages of desired size (e.g. /dev/hugepages), $id is domain ID
and $shortName is shortened version of domain name. So for
instance, the generated path may look something like this:

  /dev/hugepages/libvirt/qemu/1-QEMUGuest

But this won't work with embed driver really, because if there
are two instances of embed driver, and they both want to start a
domain with the same name and with hugepages, both drivers will
generate the same path which is not desired. Fortunately, we can
reuse the approach for machined name generation
(v6.1.0-178-gc9bd08ee35) and include part of hash of the root in
the generated path.

Note, the important change is in qemuGetBaseHugepagePath(). The
rest is needed to pass driver around.

Signed-off-by: Michal Privoznik 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_command.c |  4 ++--
 src/qemu/qemu_conf.c| 24 +---
 src/qemu/qemu_conf.h| 10 ++
 src/qemu/qemu_driver.c  |  2 +-
 src/qemu/qemu_process.c |  2 +-
 5 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9a0a96bdea..689796a92b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3465,7 +3465,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
 if (!priv->memPrealloc)
 prealloc = true;
 } else if (useHugepage) {
-if (qemuGetDomainHupageMemPath(def, cfg, pagesize, ) < 0)
+if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, 
) < 0)
 return -1;
 if (!priv->memPrealloc)
 prealloc = true;
@@ -7251,7 +7251,7 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
 if (!pagesize &&
 qemuBuildMemoryGetDefaultPagesize(cfg, ) < 0)
 return -1;
-if (qemuGetDomainHupageMemPath(def, cfg, pagesize, _path) < 0)
+if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, _path) 
< 0)
 return -1;
 } else if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
 if (qemuGetMemoryBackingPath(def, cfg, "ram", _path) < 0)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index da2abb6188..713542f8cd 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -40,6 +40,7 @@
 #include "virxml.h"
 #include "virlog.h"
 #include "cpu/cpu.h"
+#include "domain_driver.h"
 #include "domain_nwfilter.h"
 #include "virfile.h"
 #include "virsocket.h"
@@ -1887,21 +1888,29 @@ 
qemuTranslateSnapshotDiskSourcePool(virDomainSnapshotDiskDefPtr def)
 }
 
 char *
-qemuGetBaseHugepagePath(virHugeTLBFSPtr hugepage)
+qemuGetBaseHugepagePath(virQEMUDriverPtr driver,
+virHugeTLBFSPtr hugepage)
 {
+const char *root = driver->embeddedRoot;
 char *ret;
 
-ret = g_strdup_printf("%s/libvirt/qemu", hugepage->mnt_dir);
+if (root && !STRPREFIX(hugepage->mnt_dir, root)) {
+g_autofree char * hash = virDomainDriverGenerateRootHash("qemu", root);
+ret = g_strdup_printf("%s/libvirt/%s", hugepage->mnt_dir, hash);
+} else {
+ret = g_strdup_printf("%s/libvirt/qemu", hugepage->mnt_dir);
+}
 
 return ret;
 }
 
 
 char *
-qemuGetDomainHugepagePath(const virDomainDef *def,
+qemuGetDomainHugepagePath(virQEMUDriverPtr driver,
+  const virDomainDef *def,
   virHugeTLBFSPtr hugepage)
 {
-g_autofree char *base = qemuGetBaseHugepagePath(hugepage);
+g_autofree char *base = qemuGetBaseHugepagePath(driver, hugepage);
 g_autofree char *domPath = virDomainDefGetShortName(def);
 char *ret = NULL;
 
@@ -1920,11 +1929,12 @@ qemuGetDomainHugepagePath(const virDomainDef *def,
  *-1 otherwise.
  */
 int
-qemuGetDomainHupageMemPath(const virDomainDef *def,
-   virQEMUDriverConfigPtr cfg,
+qemuGetDomainHupageMemPath(virQEMUDriverPtr driver,
+   const virDomainDef *def,
unsigned long long pagesize,
char **memPath)
 {
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 size_t i = 0;
 
 if (!cfg->nhugetlbfs) {
@@ -1947,7 +1957,7 @@ qemuGetDomainHupageMemPath(const virDomainDef *def,
 return -1;
 }
 
-if (!(*memPath = qemuGetDomainHugepagePath(def, >hugetlbfs[i])))
+if (!(*memPath = qemuGetDomainHugepagePath(driver, def, 
>hugetlbfs[i])))
 return -1;
 
 return 0;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 8a0d220ce7..b85a9497b7 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -386,12 +386,14 @@ virDomainXMLOptionPtr 
virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver,
 
 int qemuTranslateSnapshotDiskSourcePool(virDomainSnapshotDiskDefPtr def);
 
-char * 

[PATCH v3 07/10] Revert "qemu_conf: Track embed root dir"

[Michal Privoznik]

This reverts commit 06a19921b6a522cd7b4d352c9320909752947de3.

What I haven't realized when writing this ^^ commit is that the
virQEMUDriver structure already stores the root directory path.
And since the pointer is immutable it can be accessed right from
the structure and thus there is no need to duplicate it in the
driver config.

Signed-off-by: Michal Privoznik 
Reviewed-by: Andrea Bolognani 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_conf.c | 2 --
 src/qemu/qemu_conf.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5339c5fc04..da2abb6188 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -115,7 +115,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged,
 
 if (root) {
 cfg->uri = g_strdup_printf("qemu:///embed?root=%s", root);
-cfg->root = g_strdup(root);
 } else {
 cfg->uri = g_strdup(privileged ? "qemu:///system" : "qemu:///session");
 }
@@ -302,7 +301,6 @@ static void virQEMUDriverConfigDispose(void *obj)
 
 virStringListFree(cfg->cgroupDeviceACL);
 VIR_FREE(cfg->uri);
-VIR_FREE(cfg->root);
 
 VIR_FREE(cfg->configBaseDir);
 VIR_FREE(cfg->configDir);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 89332eeb73..8a0d220ce7 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -76,8 +76,6 @@ struct _virQEMUDriverConfig {
 virObject parent;
 
 char *uri;
-char *root; /* The root directory for embed driver,
-   NULL for system/session connections */
 
 uid_t user;
 gid_t group;
-- 
2.24.1

[PATCH v3 04/10] conf: Move virDomainGenerateMachineName to hypervisor/

[Michal Privoznik]

The virDomainGenerateMachineName() function doesn't belong in
src/conf/ really, because it has nothing to do with domain XML
parsing. It landed there because of lack of better place in the
past. But now that we have src/hypervisor/ the function should
live there. At the same time, the function name is changed to
match new location.

Signed-off-by: Michal Privoznik 
Reviewed-by: Andrea Bolognani 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 src/conf/domain_conf.c | 72 -
 src/conf/domain_conf.h |  7 
 src/hypervisor/domain_driver.c | 74 ++
 src/hypervisor/domain_driver.h |  7 
 src/libvirt_private.syms   |  2 +-
 src/lxc/lxc_domain.c   |  3 +-
 src/qemu/qemu_domain.c |  7 ++--
 tests/virsystemdtest.c |  5 ++-
 8 files changed, 91 insertions(+), 86 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 914e03c705..efb9a61243 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -62,7 +62,6 @@
 #include "virdomainsnapshotobjlist.h"
 #include "virdomaincheckpointobjlist.h"
 #include "virutil.h"
-#include "vircrypto.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -31037,77 +31036,6 @@ virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk,
 return 0;
 }
 
-#define HOSTNAME_CHARS \
-"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-"
-
-static void
-virDomainMachineNameAppendValid(virBufferPtr buf,
-const char *name)
-{
-bool skip = true;
-
-for (; *name; name++) {
-if (strlen(virBufferCurrentContent(buf)) >= 64)
-break;
-
-if (*name == '.' || *name == '-') {
-if (!skip)
-virBufferAddChar(buf, *name);
-skip = true;
-continue;
-}
-
-skip = false;
-
-if (!strchr(HOSTNAME_CHARS, *name))
-continue;
-
-virBufferAddChar(buf, *name);
-}
-
-/* trailing dashes or dots are not allowed */
-virBufferTrimChars(buf, "-.");
-}
-
-#undef HOSTNAME_CHARS
-
-char *
-virDomainGenerateMachineName(const char *drivername,
- const char *root,
- int id,
- const char *name,
- bool privileged)
-{
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-
-virBufferAsprintf(, "%s-", drivername);
-
-if (root) {
-g_autofree char *hash = NULL;
-
-/* When two embed drivers start two domains with the same @name and @id
- * we would generate a non-unique name. Include parts of hashed @root
- * which guarantees uniqueness. The first 8 characters of SHA256 ought
- * to be enough for anybody. */
-if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, root, ) < 0)
-return NULL;
-
-virBufferAsprintf(, "embed-%.8s-", hash);
-} else if (!privileged) {
-g_autofree char *username = NULL;
-if (!(username = virGetUserName(geteuid( {
-virBufferFreeAndReset();
-return NULL;
-}
-virBufferAsprintf(, "%s-", username);
-}
-
-virBufferAsprintf(, "%d-", id);
-virDomainMachineNameAppendValid(, name);
-
-return virBufferContentAndReset();
-}
-
 
 /**
  * virDomainNetTypeSharesHostView:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 33875d942f..575290a6ac 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3649,13 +3649,6 @@ virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr 
def,
 int virDomainDiskSetBlockIOTune(virDomainDiskDefPtr disk,
 virDomainBlockIoTuneInfo *info);
 
-char *
-virDomainGenerateMachineName(const char *drivername,
- const char *root,
- int id,
- const char *name,
- bool privileged);
-
 bool
 virDomainNetTypeSharesHostView(const virDomainNetDef *net);
 
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index fc5b6eeefe..7bf0fb3f98 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -23,10 +23,84 @@
 #include "domain_driver.h"
 #include "viralloc.h"
 #include "virstring.h"
+#include "vircrypto.h"
+#include "virutil.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
 
+#define HOSTNAME_CHARS \
+"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-"
+
+static void
+virDomainMachineNameAppendValid(virBufferPtr buf,
+const char *name)
+{
+bool skip = true;
+
+for (; *name; name++) {
+if (strlen(virBufferCurrentContent(buf)) >= 64)
+break;
+
+if (*name == '.' || *name == '-') {
+if (!skip)
+virBufferAddChar(buf, *name);
+skip = true;
+continue;
+}
+
+  

[PATCH v3 03/10] qemu: Drop two layers of nesting of memoryBackingDir

[Michal Privoznik]

Initially introduced in v3.10.0-rc1~172.

When generating a path for memory-backend-file or -mem-path, qemu
driver will use the following pattern:

  $memoryBackingDir/libvirt/qemu/$id-$shortName

where $memoryBackingDir defaults to /var/lib/libvirt/qemu/ram but
can be overridden in qemu.conf. Anyway, the "/libvirt/qemu/" part
looks redundant, because it's already contained in the default,
or creates unnecessary nesting if overridden in qemu.conf.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_conf.c  | 25 ++-
 src/qemu/qemu_conf.h  |  2 --
 src/qemu/qemu_driver.c| 12 +
 .../qemuxml2argvdata/cpu-numa-memshared.args  |  8 +++---
 .../fd-memory-no-numa-topology.args   |  2 +-
 .../fd-memory-numa-topology.args  |  4 +--
 .../fd-memory-numa-topology2.args |  8 +++---
 .../fd-memory-numa-topology3.args | 12 -
 .../hugepages-memaccess2.args | 12 -
 .../qemuxml2argvdata/pages-dimm-discard.args  |  4 +--
 ...vhost-user-fs-fd-memory.x86_64-latest.args |  2 +-
 11 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5ac316ec77..5339c5fc04 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -970,7 +970,18 @@ static int
 virQEMUDriverConfigLoadMemoryEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
 {
-return virConfGetValueString(conf, "memory_backing_dir", 
>memoryBackingDir);
+char *dir = NULL;
+int rc;
+
+if ((rc = virConfGetValueString(conf, "memory_backing_dir", )) < 0) {
+return -1;
+} else if (rc > 0) {
+VIR_FREE(cfg->memoryBackingDir);
+cfg->memoryBackingDir = g_strdup_printf("%s/libvirt/qemu", dir);
+return 1;
+}
+
+return 0;
 }
 
 
@@ -1945,27 +1956,17 @@ qemuGetDomainHupageMemPath(const virDomainDef *def,
 }
 
 
-void
-qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg,
- char **path)
-{
-*path = g_strdup_printf("%s/libvirt/qemu", cfg->memoryBackingDir);
-}
-
-
 int
 qemuGetMemoryBackingDomainPath(const virDomainDef *def,
virQEMUDriverConfigPtr cfg,
char **path)
 {
 g_autofree char *shortName = NULL;
-g_autofree char *base = NULL;
 
 if (!(shortName = virDomainDefGetShortName(def)))
 return -1;
 
-qemuGetMemoryBackingBasePath(cfg, );
-*path = g_strdup_printf("%s/%s", base, shortName);
+*path = g_strdup_printf("%s/%s", cfg->memoryBackingDir, shortName);
 
 return 0;
 }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 10bc7e4a52..89332eeb73 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -397,8 +397,6 @@ int qemuGetDomainHupageMemPath(const virDomainDef *def,
unsigned long long pagesize,
char **memPath);
 
-void qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg,
-  char **path);
 int qemuGetMemoryBackingDomainPath(const virDomainDef *def,
virQEMUDriverConfigPtr cfg,
char **path);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 49dcd0e82d..716b82f8f2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -640,7 +640,6 @@ qemuStateInitialize(bool privileged,
 virQEMUDriverConfigPtr cfg;
 uid_t run_uid = -1;
 gid_t run_gid = -1;
-g_autofree char *memoryBackingPath = NULL;
 bool autostart = true;
 size_t i;
 const char *defsecmodel = NULL;
@@ -935,17 +934,8 @@ qemuStateInitialize(bool privileged,
 goto error;
 }
 
-qemuGetMemoryBackingBasePath(cfg, );
-
-if (virFileMakePath(memoryBackingPath) < 0) {
-virReportSystemError(errno,
- _("unable to create memory backing path %s"),
- memoryBackingPath);
-goto error;
-}
-
 if (privileged &&
-virFileUpdatePerm(memoryBackingPath,
+virFileUpdatePerm(cfg->memoryBackingDir,
   0, S_IXGRP | S_IXOTH) < 0)
 goto error;
 
diff --git a/tests/qemuxml2argvdata/cpu-numa-memshared.args 
b/tests/qemuxml2argvdata/cpu-numa-memshared.args
index 752eed8d13..8e214189db 100644
--- a/tests/qemuxml2argvdata/cpu-numa-memshared.args
+++ b/tests/qemuxml2argvdata/cpu-numa-memshared.args
@@ -15,12 +15,12 @@ QEMU_AUDIO_DRV=none \
 -realtime mlock=off \
 -smp 16,sockets=2,cores=4,threads=2 \
 -object memory-backend-file,id=ram-node0,\
-mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
-share=yes,size=112197632 \
+mem-path=/var/lib/libvirt/qemu/ram/-1-QEMUGuest1/ram-node0,share=yes,\
+size=112197632 \
 -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
 -object memory-backend-file,id=ram-node1,\

[PATCH v3 01/10] tests: Fix virQEMUDriverConfigNew() calling with respect to @root

[Michal Privoznik]

The virQEMUDriverConfigNew() accepts path to root directory for
embed mode as an argument. If the argument is not NULL it uses
the passed value as prefix for some internal paths (e.g.
cfg->libDir). If it is NULL though, it looks if the other
argument, @privileged is true or false and generates internal
paths accordingly. But when calling the function from the test
suite, instead of passing NULL for @root, an empty string is
passed. Fortunately, this doesn't create a problem because in
both problematic cases the generated paths are "fixed" to point
somewhere into build dir or the code which is tested doesn't
access them.

Signed-off-by: Michal Privoznik 
Reviewed-by: Andrea Bolognani 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Daniel P. Berrangé 
---
 tests/domaincapstest.c | 2 +-
 tests/testutilsqemu.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index fb803eaa47..c3a9f4ef91 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -369,7 +369,7 @@ mymain(void)
 #endif
 
 #if WITH_QEMU
-virQEMUDriverConfigPtr cfg = virQEMUDriverConfigNew(false, "");
+virQEMUDriverConfigPtr cfg = virQEMUDriverConfigNew(false, NULL);
 
 if (!cfg)
 return EXIT_FAILURE;
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index f3b4e2b3b2..cb68ac0488 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -390,7 +390,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
 return -1;
 
 driver->hostarch = virArchFromHost();
-driver->config = virQEMUDriverConfigNew(false, "");
+driver->config = virQEMUDriverConfigNew(false, NULL);
 if (!driver->config)
 goto error;
 
-- 
2.24.1

[PATCH v3 02/10] qemu: Drop virQEMUDriverIsPrivileged()

[Michal Privoznik]

Introduced in v1.2.17-rc1~121, the assumption was that the
driver->privileged is immutable at the time but it might change
in the future. Well, it did not ever since. It is still immutable
variable. Drop the needless accessor then.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_cgroup.c|  4 ++--
 src/qemu/qemu_command.c   |  4 ++--
 src/qemu/qemu_conf.c  |  6 --
 src/qemu/qemu_conf.h  |  1 -
 src/qemu/qemu_domain.c| 12 ++--
 src/qemu/qemu_driver.c| 22 +++---
 src/qemu/qemu_interface.c |  6 +++---
 7 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index c0e30f6152..f1564141b6 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -926,7 +926,7 @@ qemuInitCgroup(virDomainObjPtr vm,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(priv->driver);
 
-if (!virQEMUDriverIsPrivileged(priv->driver))
+if (!priv->driver->privileged)
 goto done;
 
 if (!virCgroupAvailable())
@@ -1061,7 +1061,7 @@ qemuConnectCgroup(virDomainObjPtr vm)
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(priv->driver);
 int ret = -1;
 
-if (!virQEMUDriverIsPrivileged(priv->driver))
+if (!priv->driver->privileged)
 goto done;
 
 if (!virCgroupAvailable())
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d1b689dfd3..9a0a96bdea 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8102,7 +8102,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 /* network and bridge use a tap device, and direct uses a
  * macvtap device
  */
-if (virQEMUDriverIsPrivileged(driver) && nicindexes && nnicindexes &&
+if (driver->privileged && nicindexes && nnicindexes &&
 net->ifname) {
 if (virNetDevGetIndex(net->ifname, ) < 0 ||
 VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex) < 0)
@@ -9642,7 +9642,7 @@ qemuBuildCommandLineValidate(virQEMUDriverPtr driver,
 int spice = 0;
 int egl_headless = 0;
 
-if (!virQEMUDriverIsPrivileged(driver)) {
+if (!driver->privileged) {
 /* If we have no cgroups then we can have no tunings that
  * require them */
 
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 15837cece4..5ac316ec77 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1224,12 +1224,6 @@ virQEMUDriverConfigPtr 
virQEMUDriverGetConfig(virQEMUDriverPtr driver)
 return conf;
 }
 
-bool
-virQEMUDriverIsPrivileged(virQEMUDriverPtr driver)
-{
-return driver->privileged;
-}
-
 virDomainXMLOptionPtr
 virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver,
const char *defsecmodel)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 14f9b9e81e..10bc7e4a52 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -333,7 +333,6 @@ int
 virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg);
 
 virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver);
-bool virQEMUDriverIsPrivileged(virQEMUDriverPtr driver);
 
 virCapsHostNUMAPtr virQEMUDriverGetHostNUMACaps(virQEMUDriverPtr driver);
 virCPUDefPtr virQEMUDriverGetHostCPU(virQEMUDriverPtr driver);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index dd48b6fff3..e54eeae58f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8600,7 +8600,7 @@ qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs,
 return -1;
 
 case VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS:
-if (!virQEMUDriverIsPrivileged(driver)) {
+if (!driver->privileged) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtiofs is not yet supported in session mode"));
 return -1;
@@ -10718,7 +10718,7 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = obj->privateData;
 bool custom_hypervisor_feat = false;
 
-if (virQEMUDriverIsPrivileged(driver) &&
+if (driver->privileged &&
 (cfg->user == 0 ||
  cfg->group == 0))
 qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, 
logCtxt);
@@ -10817,7 +10817,7 @@ qemuDomainLogContextPtr 
qemuDomainLogContextNew(virQEMUDriverPtr driver,
 ctxt->path = g_strdup_printf("%s/%s.log", cfg->logDir, vm->def->name);
 
 if (cfg->stdioLogD) {
-ctxt->manager = virLogManagerNew(virQEMUDriverIsPrivileged(driver));
+ctxt->manager = virLogManagerNew(driver->privileged);
 if (!ctxt->manager)
 goto error;
 
@@ -10847,7 +10847,7 @@ qemuDomainLogContextPtr 
qemuDomainLogContextNew(virQEMUDriverPtr driver,
  * we can't rely on logrotate. We don't use O_TRUNC since
  * it is better for SELinux policy if we truncate afterwards */
 if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START &&
-

Re: [GSoC][RFC] Project Proposal: "Introducing Job control to the storage driver"

[Michal Prívozník]

On 31. 3. 2020 17:35, Prathamesh Chavan wrote:
> On Tue, Mar 31, 2020 at 3:20 PM Michal Prívozník  wrote:
>>
>> On 31. 3. 2020 2:27, Prathamesh Chavan wrote:
>>> GSoC Proposal - 2020
>>> 
>>
>> Looks good. It's visible that you've already participated in GSoC
>> previously. Just don't forget to make it final. The deadline is in ~8 hours.
>>
>> Michal
>>
> 
> Thanks for the feedback.
> I've made the final submission on the GSoC website. The updated document
> is available here [1].
> 
> I wanted to post certain questions regarding the project. Will it be okay if
> I cc'd you and Pavel to the emails and post them on the mailing list?
> Or should I be asking them on the IRC-channel?
> 
> While going through the archives I wasn't able to fin much of the GSoC project
> discussions on the mailing list. Hence, felt the need to ask this question.

Yes, discussing this on the list is fine. Usually, students write a
private e-mail to mentors, but since some ideas don't have mentors
assigned (yet), let's discuss on the list. And feel free to CC me.

Michal

Re: On the need to move to a merge request workflow

[Ján Tomko]

On a Friday in 2020, Daniel P. Berrangé wrote:

On Fri, Mar 27, 2020 at 02:15:01PM +0100, Peter Krempa wrote:

On Thu, Mar 26, 2020 at 15:50:20 +, Daniel Berrange wrote:
> On Thu, Mar 26, 2020 at 02:10:59PM +0100, Peter Krempa wrote:
> > On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
> > > We've discussed the idea of replacing our mailing list review workflow 
with
> > > a merge request workflow in various places, over the last 6 months or so,
> >
> > One thing I feel the need to voice until this is taken in place is a
> > matter of personal preference:
> >
> > I severely dislike the merge request workflow and I'll be severely
> > disappointed once we switch over to it.
>
> Can you elaborate on specific things you don't like, so we can see if
> there are any options to mitigate them ?

- it's a web page
- it's very slow on big projects and slow in general
- it's ugly, "theme" support is a joke, white background burns my
  eyes
- it works badly on a portrait display
- mangles commit messages in an attempt to render them (link and
signed-off line concatenated:

https://gitlab.com/libvirt/libvirt/-/commit/e05dd1abdc3b3eeac6e12ab105e56138d783af2a


This looks like a bug in their code which renders the messages. It is
failing to preserve a newline character when it follows a hyperlink.
I'll see about reporting this as an issue.




- usability
- default view after opening a branch is "Files" not "commits"
- web interface doesn't really carry information about which merge
  requests are new to me

- review is terrible
- threads are only single level
- response can't be quoted (okay, they can but you have to copy and
  pase what you want to quote and then select that it's a quote)
- you need to click to see the code
- comments to code are in proportional font
- a lot of useless clutter in the UI
- need to click open comments in code
- extra steps necessary to apply code locally (granted you get a
  branch by default, but fetching it is not as easy as I have with
  mail workflow)


Overall the issues are primarily focused on the web UI, rather than
the conceptual idea of the merge request workflow.


Well, many of the points in the original mail were actually against mailing
lists and not the simple merge-request-free workflow we are using thanks
to them :)


I share the same
view of many of these issues, but I do think we can mitigate most of
it with a terminal based tool as an option for those who don't like
the web UI.



- review process favours review without actually fetching the code
  locally (if you can click a button, who will actually test that
  the code works?)


If anything I feel the current email review makes me even less likely
to actually apply & test the code locally, as it requires several tedious
error prone steps, compared to being able to direct "git fetch" a ref
for the merge request.

The more automated testing we do the better, but either way we rely on
reviewers to be diligent enough to decide when something needs some
more testing.




Now I know you will suggest I try your 'bichon' tool for reviews as it
fixes a handful of the problems outlined above, but unfortunately that
comes with it's own set of problems since it's in infancy stage.


Yes, it certainly isn't developed to where it needs to be yet, but I have
started using it for real code review on libosinfo repos and found it is
useful already.



Oh it already does stuff?

Time to file some issues then because I could not get it to do anything
useful.

Jano


signature.asc
Description: PGP signature

Re: GSoC'20 Interested Student: Adding support to Jailhouse Hypervisor

[PRAKHAR BANSAL]

Hi Jan,

I have shared the draft proposal link with libvirt on GSoC's system.
Could you please check and provide your feedback, if possible.

Thanks,
Prakhar

On Tue, Mar 31, 2020 at 1:47 AM Jan Kiszka  wrote:

> On 31.03.20 04:43, PRAKHAR BANSAL wrote:
> > Hi Jan,
> >
> > Thanks for the confirmation to proceed on project proposal.
> >
> > Also, I tried installing Jailhouse on my VM after enabling VT-x/EPT and
> > IOMMU for my VM(Guest OS- Ubuntu 18.04) on VMware fusion hypervisor with
> > MacOS on the host side.
> > However,  Jailhouse hardware check was failed because of missing
> > *Preemption timer and Virtualize APIC access*, could you please suggest,
> > if this is hardware limitation?  Is there any workaround here?
>
> You will need a hypervisor that supports both when nesting, but I have
> no idea if there is one for the Mac. What is known to work is KVM on
> Linux hosts.
>
> > My laptop's processor is Intel quad-core i7.
> >
> > image.png
> >
> > Also, could you please suggest, if I can talk to you through an IRC or
> > slack channel since it is a bit hard to communicate over email every
> time.
>
> I'll be listening on #jailhouse, irc.freenode.net.
>
> Jan
>
> >
> > Thanks,
> > Prakhar
> >
> >
> > On Mon, Mar 30, 2020 at 6:15 AM Jan Kiszka  > > wrote:
> >
> > On 30.03.20 10:02, PRAKHAR BANSAL wrote:
> >  > Hi Jan,
> >  >
> >  > On Sat, Mar 28, 2020 at 4:12 AM Jan Kiszka  > 
> >  > >> wrote:
> >  >
> >  > On 28.03.20 08:47, PRAKHAR BANSAL wrote:
> >  >  > Hi Jan,
> >  >  >
> >  >  > Thanks for the reply!
> >  >  >
> >  >  > I was only considering the command-line tool "code" for
> > reference
> >  > to the
> >  >  > Jailhouse kernel API(ioctl calls) because I didn't find a
> >  > documentation
> >  >  > of the Jailhouse kernel APIs.
> >  >
> >  > Right, the IOCTL API is not documented so far. It is
> > currently only used
> >  > inside the Jailhouse project. This needs to be formalized
> > when there
> >  > shall be external users like a libvirt driver.
> >  >
> >  > That might be a nice small contribution task: Create some
> >  > Documentation/driver-interfaces.md that describes the IOCTLs
> > along with
> >  > their parameter structures and that also includes the current
> >  > sysfs-entries.txt as a section. Then send this as patch here.
> > I'll help
> >  > out when details are not clear from reading the code.
> >  >
> >  > Sure. I will do that.
> >  >
> >  >  >
> >  >  > For the second part as you mentioned that Jailhouse can
> > only create
> >  >  > cells with the constraints defined in the root cell
> > configuration. I
> >  >  > have a few questions regarding that.
> >  >  >
> >  >  > 1. Is there a way to know if Jailhouse is enabled on the
> > host and get
> >  >  > the root cell configuration(s) from Jailhouse through an
> API?
> >  > This can
> >  >  > be used while binding the libvirt to the Jailhouse
> hypervisor.
> >  >
> >  > Look at
> >  >
> >
> https://github.com/siemens/jailhouse/blob/master/Documentation/sysfs-entries.txt
> >  > for what is reported as runtime information. Full
> > configurations can't
> >  > be read back at this point. This might be reconsidered in the
> > light of
> >  > [1], but I wouldn't plat for that yet.
> >  >
> >  >
> >  > Ok, sure. I am looking into it.
> >  >
> >  >
> >  >  >
> >  >  > 2. If Jailhouse is not enabled(again can we know this
> > using some API)
> >  >  > then, can libvirt enable/disable Jailhouse during the
> libvirt
> >  > binding of
> >  >  > the Jailhouse driver with a given set of Jailhouse cell
> >  > configurations
> >  >  > describing a complete partitioned system?
> >  >
> >  > With the API above and a given configuration set, yes. The
> > config set
> >  > would have to be provided to the libvirt driver in some
> > to-be-defined
> >  > way (e.g. /etc/libvirt/jailhouse.conf ->
> > /etc/libvirt/jailhouse/*.cell).
> >  >
> >  > Cool, got it. Thanks!
> >  >
> >  >  >
> >  >  > 3. I was wondering, as you mentioned that libvirt driver
> > should check
> >  >  > for mismatch of the cell configuration with the root cell
> >  > configuration,
> >  >  > the question is, isn't that done by Jailhouse itself? If
> > yes, then
> >  >  > libvirt can just pass on the cell creation requests to
> > Jailhouse and
> >  >  > return the response to the user as it is, rather than
> driver
> >  > doing any

[PATCH] qemu_cgroup.c: use VIR_AUTOSTRINGLIST, g_autofree and g_autoptr

[Seeteena Thoufeek]

Signed-off-by: Seeteena Thoufeek 
---
 src/qemu/qemu_cgroup.c | 91 +++---
 1 file changed, 35 insertions(+), 56 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index c0e30f6..d34c515 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -62,7 +62,7 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 int perms = VIR_CGROUP_DEVICE_READ;
-char **targetPaths = NULL;
+VIR_AUTOSTRINGLIST targetPaths = NULL;
 size_t i;
 int rv;
 int ret = -1;
@@ -82,12 +82,11 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
  virCgroupGetDevicePermsString(perms),
  rv);
 if (rv < 0)
-goto cleanup;
+return ret;
 
 if (rv > 0) {
 /* @path is neither character device nor block device. */
-ret = 0;
-goto cleanup;
+return 0;
 }
 
 if (virDevMapperGetTargets(path, ) < 0 &&
@@ -95,7 +94,7 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
 virReportSystemError(errno,
  _("Unable to get devmapper targets for %s"),
  path);
-goto cleanup;
+return ret;
 }
 
 for (i = 0; targetPaths && targetPaths[i]; i++) {
@@ -105,13 +104,10 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
  virCgroupGetDevicePermsString(perms),
  rv);
 if (rv < 0)
-goto cleanup;
+return ret;
 }
 
-ret = 0;
- cleanup:
-virStringListFree(targetPaths);
-return ret;
+return 0;
 }
 
 
@@ -739,7 +735,7 @@ static int
 qemuSetupDevicesCgroup(virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
-virQEMUDriverConfigPtr cfg = NULL;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
 const char *const *deviceACL = NULL;
 int rv = -1;
 int ret = -1;
@@ -757,15 +753,15 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
 return 0;
 }
 
-goto cleanup;
+return ret;
 }
 
 if (qemuSetupFirmwareCgroup(vm) < 0)
-goto cleanup;
+return ret;
 
 for (i = 0; i < vm->def->ndisks; i++) {
 if (qemuSetupImageChainCgroup(vm, vm->def->disks[i]->src) < 0)
-goto cleanup;
+return ret;
 }
 
 rv = virCgroupAllowDevice(priv->cgroup, 'c', DEVICE_PTY_MAJOR, -1,
@@ -773,9 +769,8 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
 virDomainAuditCgroupMajor(vm, priv->cgroup, "allow", DEVICE_PTY_MAJOR,
   "pty", "rw", rv == 0);
 if (rv < 0)
-goto cleanup;
+return ret;
 
-cfg = virQEMUDriverGetConfig(priv->driver);
 deviceACL = cfg->cgroupDeviceACL ?
 (const char *const *)cfg->cgroupDeviceACL :
 defaultDeviceACL;
@@ -791,7 +786,7 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
 virDomainAuditCgroupMajor(vm, priv->cgroup, "allow", DEVICE_SND_MAJOR,
   "sound", "rw", rv == 0);
 if (rv < 0)
-goto cleanup;
+return ret;
 }
 
 for (i = 0; deviceACL[i] != NULL; i++) {
@@ -805,57 +800,54 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
 virDomainAuditCgroupPath(vm, priv->cgroup, "allow", deviceACL[i], 
"rw", rv);
 if (rv < 0 &&
 !virLastErrorIsSystemErrno(ENOENT))
-goto cleanup;
+return ret;
 }
 
 if (virDomainChrDefForeach(vm->def,
true,
qemuSetupChardevCgroupCB,
vm) < 0)
-goto cleanup;
+return ret;
 
 if (vm->def->tpm && qemuSetupTPMCgroup(vm) < 0)
-goto cleanup;
+return ret;
 
 for (i = 0; i < vm->def->nhostdevs; i++) {
 /* This may allow /dev/vfio/vfio multiple times, but that
  * is not a problem. Kernel will have only one record. */
 if (qemuSetupHostdevCgroup(vm, vm->def->hostdevs[i]) < 0)
-goto cleanup;
+return ret;
 }
 
 for (i = 0; i < vm->def->nmems; i++) {
 if (qemuSetupMemoryDevicesCgroup(vm, vm->def->mems[i]) < 0)
-goto cleanup;
+return ret;
 }
 
 for (i = 0; i < vm->def->ngraphics; i++) {
 if (qemuSetupGraphicsCgroup(vm, vm->def->graphics[i]) < 0)
-goto cleanup;
+return ret;
 }
 
 for (i = 0; i < vm->def->nvideos; i++) {
 if (qemuSetupVideoCgroup(vm, vm->def->videos[i]) < 0)
-goto cleanup;
+return ret;
 }
 
 for (i = 0; i < vm->def->ninputs; i++) {
 if (qemuSetupInputCgroup(vm, vm->def->inputs[i]) < 0)
-goto cleanup;
+return ret;
 }
 
 for (i = 0; i < vm->def->nrngs; i++) {
 if (qemuSetupRNGCgroup(vm, 

Re: [libvirt-jenkins-ci PATCH 5/5] guests: lcitool: Enable the new 'gitlab' flavor in the lcitool script

[Andrea Bolognani]

On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
> +++ b/guests/lcitool
> +def get_gitlab_runner_token_file(self):
> +gitlab_runner_token_file = 
> self._get_config_file("gitlab-runner-token")
> +
> +try:
> +with open(gitlab_runner_token_file, "r") as infile:
> +if not infile.readline().strip():
> +raise ValueError
> +except Exception as ex:
> +raise Exception(
> +"Missing or invalid gitlab runner token file ({}): 
> {}".format(
> +gitlab_runner_token_file, ex
> +)
> +)
> +
> +return gitlab_runner_token_file

As per my comments to patch 3/5, we want the token to be stored in
the vault instead that on the disk, which will make most of this
commit not necessary.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt-jenkins-ci PATCH 4/5] playbooks: gitlab: Force a random password for the root account

[Andrea Bolognani]

On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
> Unlike with the 'test' flavour, where the 'test' user has sudo
> permissions on the system, with machines set up with the 'gitlab'
> flavour which are intended to contact the outside world which, we don't
> want that. More importantly though, we must not use the default root
> password which is set by the install script on such machines.
> Therefore, set the root password to a random one as part of the gitlab
> flavour task, thus only allowing SSH pubkey authentication for the root
> account.

I'm confused by this.

If we want the root account to only be accessible via SSH with a
pubkey, then we can configure sshd accordingly: setting a random
password which is not stored anywhere prevents access not only via
SSH, but also via local access (eg. serial console), which I don't
think is desirable.

Moreover, the root password that is set in the first place is taken
from a mandatory user-provided configuration file, and I'm not sure
we should be condescending towards users by basically saying "we know
you didn't choose a secure password, so we're going to generate a new
one ourselves".

What am I missing?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [GSoC][RFC] Project Proposal: "Introducing Job control to the storage driver"

[Prathamesh Chavan]

On Tue, Mar 31, 2020 at 3:20 PM Michal Prívozník  wrote:
>
> On 31. 3. 2020 2:27, Prathamesh Chavan wrote:
> > GSoC Proposal - 2020
> > 
>
> Looks good. It's visible that you've already participated in GSoC
> previously. Just don't forget to make it final. The deadline is in ~8 hours.
>
> Michal
>

Thanks for the feedback.
I've made the final submission on the GSoC website. The updated document
is available here [1].

I wanted to post certain questions regarding the project. Will it be okay if
I cc'd you and Pavel to the emails and post them on the mailing list?
Or should I be asking them on the IRC-channel?

While going through the archives I wasn't able to fin much of the GSoC project
discussions on the mailing list. Hence, felt the need to ask this question.

Thanks,
Prathamesh Chavan

[1]: docs.google.com/document/d/1Js-yi1oNrl9fRhzvMJwBHyygYCAtr_q7Bby8k1jOdig

Re: [libvirt-jenkins-ci PATCH 3/5] guests: Introduce the new 'gitlab' flavor

[Andrea Bolognani]

On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
> +++ b/guests/group_vars/all/main.yml
> @@ -5,3 +5,6 @@
>  ansible_ssh_pass: root
>  
>  jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname 
> }}/slave-agent.jnlp
> +
> +# In our case, ansible_system is either Linux or FreeBSD
> +gitlab_runner_url: 
> https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{
>  ansible_system|lower }}-amd64

Note that these two URLs are semantically quite different:

  * jenkins_url is the endpoint to which the Jenkins agent will
connect to;

  * gitlab_runner_url is the location from where the gitlab-runner
binary can be downloaded.

To keep things consistent, we could have

  jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname 
}}/slave-agent.jnlp
  jenkins_agent_url: https://ci.centos.org/jnlpJars/slave.jar

  gitlab_url: https://gitlab.com
  gitlab_runner_url: 
https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{
 ansible_system|lower }}-amd64

although I'm starting to question why these URLs should even be part
of the inventory when they could just as easily be hardcoded into the
corresponding playbook...

> +++ b/guests/playbooks/update/tasks/gitlab.yml
> @@ -0,0 +1,64 @@
> +---
> +- name: Look up Gitlab runner secret
> +  set_fact:
> +gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}'

The GitLab token should be extracted from the vault, not from a
regular file.

[...]
> +- name: Make sure the gitlab-runner config dir exists exists
> +  file:
> +path: '{{ gitlab_runner_config_path | dirname }}'
> +owner: gitlab
> +group: gitlab
> +state: directory
> +  register: rc_gitlab_runner_config_dir
> +
> +- name: Create and empty gitlab-runner config
> +  file:
> +path: '{{ gitlab_runner_config_path }}'
> +owner: gitlab
> +group: gitlab
> +state: touch
> +  when: rc_gitlab_runner_config_dir.changed

Running 'gitlab-runner register' will create the configuration file,
so touching it in advance is unnecessary and...

> +# To ensure idempotency, we must run the registration only when we first
> +# created the config dir, otherwise we'll register a new runner on every
> +# update
> +- name: Register the gitlab-runner agent
> +  shell: 'gitlab-runner register --non-interactive --config 
> /home/gitlab/.gitlab-runner/config.toml --registration-token {{ 
> gitlab_runner_secret }} --url https://gitlab.com --executor shell --tag-list 
> {{ inventory_hostname }}'
> +  when: rc_gitlab_runner_config_dir.changed

... this check could be replaced with

  creates: '{{ gitlab_runner_config_path }}'

Additional comments on this command:

  * you have defined gitlab_runner_config_path earlier, might as well
use it here;

  * the 'shell' executor is I think our only option when it comes to
FreeBSD (and we should make sure we fully understand the security
implications of using it before exposing any of this on the
internet), but for Linux we should be able to use the 'docker'
executor instead, which should provide additional isolation;

  * I haven't played around with tags much, but from what I think I
have understood we will want to have something more generic here,
such as "freebsd" and "linux", so that we can for example tag
some of the jobs as "freebsd" and be sure they will run on the
expected OS;

  * I think we'll want to have the Linux runners configured to accept
untagged jobs, but the FreeBSD ones only accepting tagged ones
(again, if I have understood how job tagging works).

[...]
> +- block:
> +- name: Install the gitlab_runner rc service script
> +  template:
> +src: '{{ playbook_base }}/templates/gitlab-runner.j2'
> +dest: '/usr/local/etc/rc.d/gitlab_runner'
> +mode: '0755'
> +
> +- name: Enable the gitlab-runner rc service

s/gitlab-runner/gitlab_runner/

though I wonder if we can't just use the same name on both Linux and
FreeBSD? On my FreeBSD machine, while the use of underscore in
service names is clearly the default, there's at least one service
(ftp-proxy) which doesn't follow that convention with (I assume) no
ill effect.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt-jenkins-ci PATCH 2/5] guests: templates: Introduce a gitlab-runner RC init service template

[Andrea Bolognani]

On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
> guests: templates: Introduce a gitlab-runner RC init service template

"RC init" is sort of a loaded term, I'd just call out FreeBSD
directly.

> +++ b/guests/playbooks/update/templates/gitlab-runner.j2
> @@ -0,0 +1,32 @@
> +#!/bin/sh
> +# PROVIDE: gitlab_runner
> +# REQUIRE: DAEMON NETWORKING
> +# BEFORE:
> +# KEYWORD:

This seems to be heavily based on [1], so maybe include a reference
to that URL somewhere.

> +user="{{ flavor }}"
> +user_home="/home/{{ flavor }}"

Either use substitution for {{ flavor }} both here and in the systemd
service, or in neither. Personally I'd go for the latter, since it's
not really buying us much.

> +gitlab_runner_start()
> +{
> +export USER=${user}
> +export HOME=${user_home}
> +export PATH=${PATH}:/usr/local/bin/:/usr/local/sbin/
> +if checkyesno ${rcvar}; then
> +cd ${user_home}
> +/usr/sbin/daemon -p ${pidfile} ${command} ${command_args} > 
> /var/log/gitlab-runner.log 2>&1

The version in the official documentation does this a little
differently... I guess the difference is that in their case the
gitlab-runner application is running as the gitlab user, wereas in
ours the daemon is running as root but is instructed to execute
workloads as the gitlab user. The latter seems fine, as that's what
happens on Linux as well, but have you fully considered the security
implications?


[1] https://docs.gitlab.com/runner/install/freebsd.html
-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt-jenkins-ci PATCH 1/5] guests: templates: Introduce a gitlab-runner systemd service template

[Andrea Bolognani]

On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
> +++ b/guests/playbooks/update/templates/gitlab-runner.service.j2
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=GitLab Runner
> +After=network.target
> +ConditionFileIsExecutable=/usr/local/bin/gitlab-runner
> +
> +[Service]
> +ExecStart=/usr/local/bin/gitlab-runner run --user gitlab --working-directory 
> /home/gitlab --config /home/gitlab/.gitlab-runner/config.toml

Since we're going to use /home/gitlab as the base directory for all
things gitlab-runner, I would suggest we store the gitlab-runner
binary itself inside that directory too, and since we're pointing
the application to the configuration file explicitly we could
shorten that a bit and use /home/gitlab/config.toml as the path.

The only setting that concerns me a bit is --working-directory: is
that the base directory for gitlab-runner's operation, or is that
the one where it will store source code and temporary files? Because
in the latter case I would feel more comfortable if we used a
subdirectory of /home/gitlab, rather than the home directory itself,
as path.

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt-jenkins-ci PATCH] guests: allow for container image inheritance

[Daniel P . Berrangé]

Currently when creating a Dockerfile for a container, we include the
full set of base packages, along with the packages for the project
itself. If building a Perl binding, this would require us to install
the base package, libvirt packages and Perl packages. With the use
of the "--inherit libvirt-fedora-30" arg, it is possible to have a
dockerfile that only adds the Perl packages, getting everything
else from the parent image.

For example, a full Dockerfile for libvirt-go would thus be:

  FROM libvirt-centos-7:latest

  RUN yum install -y \
  golang && \
  yum autoremove -y && \
  yum clean all -y

Note there is no need to set ENV either, as that's inherited from the
parent container.

Signed-off-by: Daniel P. Berrangé 
---
 guests/lcitool | 108 -
 1 file changed, 98 insertions(+), 10 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 689a8cf..b3afb6a 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -396,6 +396,12 @@ class Application:
 help="target architecture for cross compiler",
 )
 
+def add_inherit_arg(parser):
+parser.add_argument(
+"-i", "--inherit",
+help="inherit from an intermediate container image",
+)
+
 def add_wait_arg(parser):
 parser.add_argument(
 "-w", "--wait",
@@ -442,6 +448,7 @@ class Application:
 add_hosts_arg(dockerfileparser)
 add_projects_arg(dockerfileparser)
 add_cross_arch_arg(dockerfileparser)
+add_inherit_arg(dockerfileparser)
 
 def _execute_playbook(self, playbook, hosts, projects, git_revision):
 base = Util.get_base()
@@ -644,11 +651,11 @@ class Application:
 with open(keyfile, "r") as r:
 return r.read().rstrip()
 
-def _dockerfile_build_varmap(self, facts, mappings, pip_mappings, 
projects, cross_arch):
+def _dockerfile_build_varmap(self, facts, mappings, pip_mappings, 
projects, cross_arch, needbase):
 if facts["package_format"] == "deb":
-varmap = self._dockerfile_build_varmap_deb(facts, mappings, 
pip_mappings, projects, cross_arch)
+varmap = self._dockerfile_build_varmap_deb(facts, mappings, 
pip_mappings, projects, cross_arch, needbase)
 if facts["package_format"] == "rpm":
-varmap = self._dockerfile_build_varmap_rpm(facts, mappings, 
pip_mappings, projects, cross_arch)
+varmap = self._dockerfile_build_varmap_rpm(facts, mappings, 
pip_mappings, projects, cross_arch, needbase)
 
 varmap["package_manager"] = facts["package_manager"]
 varmap["cc"] = facts["cc"]
@@ -662,7 +669,7 @@ class Application:
 
 return varmap
 
-def _dockerfile_build_varmap_deb(self, facts, mappings, pip_mappings, 
projects, cross_arch):
+def _dockerfile_build_varmap_deb(self, facts, mappings, pip_mappings, 
projects, cross_arch, needbase):
 package_format = facts["package_format"]
 package_manager = facts["package_manager"]
 os_name = facts["os_name"]
@@ -682,7 +689,10 @@ class Application:
 
 # We need to add the base project manually here: the standard
 # machinery hides it because it's an implementation detail
-for project in projects + ["base"]:
+pkgprojects = projects
+if needbase:
+pkgprojects = projects + ["base"]
+for project in pkgprojects:
 for package in self._projects.get_packages(project):
 cross_policy = "native"
 for key in cross_keys:
@@ -730,7 +740,7 @@ class Application:
 
 return varmap
 
-def _dockerfile_build_varmap_rpm(self, facts, mappings, pip_mappings, 
projects, cross_arch):
+def _dockerfile_build_varmap_rpm(self, facts, mappings, pip_mappings, 
projects, cross_arch, needbase):
 package_format = facts["package_format"]
 package_manager = facts["package_manager"]
 os_name = facts["os_name"]
@@ -744,7 +754,10 @@ class Application:
 
 # We need to add the base project manually here: the standard
 # machinery hides it because it's an implementation detail
-for project in projects + ["base"]:
+pkgprojects = projects
+if needbase:
+pkgprojects = projects + ["base"]
+for project in pkgprojects:
 for package in self._projects.get_packages(project):
 for key in keys:
 if key in mappings[package]:
@@ -791,7 +804,7 @@ class Application:
 
 return varmap
 
-def _dockerfile_format(self, facts, cross_arch, varmap):
+def _dockerfile_base_format(self, facts, cross_arch, varmap):
 package_format = facts["package_format"]
 package_manager = facts["package_manager"]
 os_name = facts["os_name"]
@@ -931,6 +944,74 @@ class Application:
 ENV CONFIGURE_OPTS "--host={cross_abi}"
 

[libvirt-go-xml PATCH 2/2] travis: delete CI configuration

[Daniel P . Berrangé]

We are standardizing on GitLab, so no longer need to run CI tests on
Travis.

Signed-off-by: Daniel P. Berrangé 
---
 .travis.yml | 13 -
 1 file changed, 13 deletions(-)
 delete mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index f3396ba..000
--- a/.travis.yml
+++ /dev/null
@@ -1,13 +0,0 @@
-language: go
-os: linux
-dist: trusty
-
-go:
-  - 1.5
-  - 1.6
-  - 1.7
-  - 1.8
-  - 1.9
-
-script:
-  go test -timeout 10m -tags xmlroundtrip -v
-- 
2.24.1

[libvirt-go-xml PATCH 0/2] Switch CI from Travis to GitLab

[Daniel P . Berrangé]

Daniel P. Berrangé (2):
  gitlab: add CI definition for GitLab
  travis: delete CI configuration

 .gitlab-ci.yml | 25 +
 .travis.yml| 13 -
 README.md  |  2 +-
 3 files changed, 26 insertions(+), 14 deletions(-)
 create mode 100644 .gitlab-ci.yml
 delete mode 100644 .travis.yml

-- 
2.24.1

[libvirt-go-xml PATCH 1/2] gitlab: add CI definition for GitLab

[Daniel P . Berrangé]

We want to test against a variety of Go versions to validate that there
are not any incompatibilities in XML parser handling between versions.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.yml | 25 +
 README.md  |  2 +-
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 .gitlab-ci.yml

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 000..f19d240
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,25 @@
+
+.go_build: _build
+  script:
+- go test -timeout 10m -tags xmlroundtrip -v
+
+
+go_1_10:
+  <<: *go_build
+  image: golang:1.10
+
+go_1_11:
+  <<: *go_build
+  image: golang:1.11
+
+go_1_12:
+  <<: *go_build
+  image: golang:1.12
+
+go_1_13:
+  <<: *go_build
+  image: golang:1.13
+
+go_1_14:
+  <<: *go_build
+  image: golang:1.14
diff --git a/README.md b/README.md
index fcc1ffb..9e39190 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-# libvirt-go-xml [![Build 
Status](https://travis-ci.org/libvirt/libvirt-go-xml.svg?branch=master)](https://travis-ci.org/libvirt/libvirt-go-xml)
 
[![GoDoc](https://godoc.org/libvirt.org/libvirt-go-xml?status.svg)](https://godoc.org/libvirt.org/libvirt-go-xml)
+# libvirt-go-xml [![build 
status](https://gitlab.com/berrange/libvirt-go-xml/badges/master/pipeline.svg)](https://gitlab.com/berrange/libvirt-go-xml/-/commits/master)
 
[![GoDoc](https://godoc.org/libvirt.org/libvirt-go-xml?status.svg)](https://godoc.org/libvirt.org/libvirt-go-xml)
 
 Go API for manipulating libvirt XML documents
 
-- 
2.24.1

Re: [libvirt PATCH] travis: delete all Linux jobs

[Daniel P . Berrangé]

On Tue, Mar 31, 2020 at 02:21:00PM +0200, Andrea Bolognani wrote:
> On Mon, 2020-03-30 at 18:12 +0100, Daniel P. Berrangé wrote:
> > On Mon, Mar 30, 2020 at 07:05:26PM +0200, Andrea Bolognani wrote:
> > > My only concern is that our ci/Makefile scaffolding will bitrot now
> > > that it will no longer be exercised directly through CI... Perhaps we
> > > could leave a single Linux job enabled on Travis CI for the sole
> > > purpose of preventing that?
> > 
> > True, but I'd rather like to eliminate duplicate failures being reported
> > by Travis that we're already getting from GitLab, and thus stop loooking
> > at Travis as much as is possible, so if it fails we know it is macOS
> > related.
> 
> Since we're going to be forced to keep the macOS job on Travis, we
> will still ultimately have to look in two places... Additionally,
> while right now CentOS CI gives you an overview of the CI status of
> all jobs, from libvirt all the way to virt-manager, once all projects
> have been moved to GitLab CI we're going to lose that.
> 
> Could we create a sort of high-level CI dashboard that provides an
> at-a-glance status? It doesn't have to be very detailed, it could be
> basically just a single colored dot per project to signal its overall
> status, along with links pointing to the appropriate pages for when
> additional details are necessary.

That's doable with a combination of GitLab CI + Pages I expect.
A script would fetch the job status from all the relevant places
and render a static HTML page. We publish that to gitlab Pages,
and setup a scheduled CI pipeline to refresh once an hour or so.

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

Re: [libvirt PATCH] travis: delete all Linux jobs

[Andrea Bolognani]

On Mon, 2020-03-30 at 18:12 +0100, Daniel P. Berrangé wrote:
> On Mon, Mar 30, 2020 at 07:05:26PM +0200, Andrea Bolognani wrote:
> > My only concern is that our ci/Makefile scaffolding will bitrot now
> > that it will no longer be exercised directly through CI... Perhaps we
> > could leave a single Linux job enabled on Travis CI for the sole
> > purpose of preventing that?
> 
> True, but I'd rather like to eliminate duplicate failures being reported
> by Travis that we're already getting from GitLab, and thus stop loooking
> at Travis as much as is possible, so if it fails we know it is macOS
> related.

Since we're going to be forced to keep the macOS job on Travis, we
will still ultimately have to look in two places... Additionally,
while right now CentOS CI gives you an overview of the CI status of
all jobs, from libvirt all the way to virt-manager, once all projects
have been moved to GitLab CI we're going to lose that.

Could we create a sort of high-level CI dashboard that provides an
at-a-glance status? It doesn't have to be very detailed, it could be
basically just a single colored dot per project to signal its overall
status, along with links pointing to the appropriate pages for when
additional details are necessary.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] Add blog for ARM Datacenter Project.

[Robert Foley]

Fantastic, thank you.

Regards,
-Rob

On Tue, 31 Mar 2020 at 04:43, Daniel P. Berrangé  wrote:
>
> On Mon, Mar 30, 2020 at 02:39:36PM -0400, Robert Foley wrote:
> > Our blog covers QEMU topics.
> >
> > Signed-off-by: Robert Foley 
> > ---
> >  updater/virt-tools/config.ini | 3 +++
> >  1 file changed, 3 insertions(+)
>
> Reviewed-by: Daniel P. Berrangé 
>
> It is pushed and will be live within the next hour
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>

[virttools-web PATCH 2/3] Introduce use of GitLab CI for publishing to GitLab Pages

[Daniel P . Berrangé]

To publish to GitLab pages, all that is required is to provide a CI job
that creates files in a directory called "public" and list that as an
artifact.

When the CI job completes, the website is immediately available at the
URL  https://username.gitlab.io/reponame. This makes it much easier to
preview changes to the site than with OpenShift apps.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.yml|   9 +
 README.md |  14 --
 {web => public}/header-bg.png | Bin
 {web => public}/index.css |   0
 {web => public}/index.html|   4 +---
 {web => public}/logo-kvm.png  | Bin
 {web => public}/logo-libguestfs.png   | Bin
 {web => public}/logo-libosinfo.png| Bin
 {web => public}/logo-libvirt.png  | Bin
 {web => public}/logo-qemu.png | Bin
 {web => public}/logo-virt-manager.png | Bin
 {web => public}/logo.png  | Bin
 {web => public}/logo.xcf  | Bin
 13 files changed, 14 insertions(+), 13 deletions(-)
 create mode 100644 .gitlab-ci.yml
 rename {web => public}/header-bg.png (100%)
 rename {web => public}/index.css (100%)
 rename {web => public}/index.html (93%)
 rename {web => public}/logo-kvm.png (100%)
 rename {web => public}/logo-libguestfs.png (100%)
 rename {web => public}/logo-libosinfo.png (100%)
 rename {web => public}/logo-libvirt.png (100%)
 rename {web => public}/logo-qemu.png (100%)
 rename {web => public}/logo-virt-manager.png (100%)
 rename {web => public}/logo.png (100%)
 rename {web => public}/logo.xcf (100%)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 000..5613e31
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,9 @@
+
+image: centos:8
+
+pages:
+  script:
+- /bin/true
+  artifacts:
+paths:
+  - public
diff --git a/README.md b/README.md
index 5b7a60d..a0c5a5e 100644
--- a/README.md
+++ b/README.md
@@ -8,14 +8,8 @@ This directory contains content / configuration for managing
 * [https://www.virttools.org](https://www.virttools.org)
 * [https://www.virt-tools.org](https://www.virt-tools.org)
 
-The site is setup to run under OpenShift
+The site is intended to be published with GitLab Pages
 
-Initial load can be done with
-
-```
-  oc process -f virttools-web/openshift/templates/virttools-web.json  | oc 
create -f -
-```
-
-Updates to the OpenShift config are manually activated using `oc replace`.
-
-Updates to the content itself are automatically propagated via a web hook.
+Upon pushing changes to the GitLab repository, CI rules will automatically
+build the site and publish the result to the repository's GitLab Pages
+site.
diff --git a/web/header-bg.png b/public/header-bg.png
similarity index 100%
rename from web/header-bg.png
rename to public/header-bg.png
diff --git a/web/index.css b/public/index.css
similarity index 100%
rename from web/index.css
rename to public/index.css
diff --git a/web/index.html b/public/index.html
similarity index 93%
rename from web/index.html
rename to public/index.html
index 3fb6f48..d801c48 100644
--- a/web/index.html
+++ b/public/index.html
@@ -92,9 +92,7 @@
 
 
   
-https://www.openshift.com/; title="Powered by OpenShift Online">
-  https://www.openshift.com/images/logos/powered_by_openshift_reverse.png;>
-
+https://gitlab.com/libvirt/virttools-web/-/blob/master/public/index.html; 
title="Edit this page">Edit this page
   
 
 
diff --git a/web/logo-kvm.png b/public/logo-kvm.png
similarity index 100%
rename from web/logo-kvm.png
rename to public/logo-kvm.png
diff --git a/web/logo-libguestfs.png b/public/logo-libguestfs.png
similarity index 100%
rename from web/logo-libguestfs.png
rename to public/logo-libguestfs.png
diff --git a/web/logo-libosinfo.png b/public/logo-libosinfo.png
similarity index 100%
rename from web/logo-libosinfo.png
rename to public/logo-libosinfo.png
diff --git a/web/logo-libvirt.png b/public/logo-libvirt.png
similarity index 100%
rename from web/logo-libvirt.png
rename to public/logo-libvirt.png
diff --git a/web/logo-qemu.png b/public/logo-qemu.png
similarity index 100%
rename from web/logo-qemu.png
rename to public/logo-qemu.png
diff --git a/web/logo-virt-manager.png b/public/logo-virt-manager.png
similarity index 100%
rename from web/logo-virt-manager.png
rename to public/logo-virt-manager.png
diff --git a/web/logo.png b/public/logo.png
similarity index 100%
rename from web/logo.png
rename to public/logo.png
diff --git a/web/logo.xcf b/public/logo.xcf
similarity index 100%
rename from web/logo.xcf
rename to public/logo.xcf
-- 
2.24.1

[virttools-web PATCH 1/3] Convert README to markdown format

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 README| 19 ---
 README.md | 21 +
 2 files changed, 21 insertions(+), 19 deletions(-)
 delete mode 100644 README
 create mode 100644 README.md

diff --git a/README b/README
deleted file mode 100644
index 9126ab1..000
--- a/README
+++ /dev/null
@@ -1,19 +0,0 @@
-  Virt Tools static site
-  ==
-
-This directory contains content / configuration for managing
-
-http(s)://virttools.org
-http(s)://virt-tools.org
-http(s)://www.virttools.org
-http(s)://www.virt-tools.org
-
-The site is setup to run under OpenShift
-
-Initial load can be done with
-
-  oc process -f virttools-web/openshift/templates/virttools-web.json  | oc 
create -f -
-
-Updates to the OpenShift config are manually activated using 'oc replace'.
-
-Updates to the content itself are automatically propagated via a web hook.
diff --git a/README.md b/README.md
new file mode 100644
index 000..5b7a60d
--- /dev/null
+++ b/README.md
@@ -0,0 +1,21 @@
+Virt Tools static site
+==
+
+This directory contains content / configuration for managing
+
+* [https://virttools.org](https://virttools.org)
+* [https://virt-tools.org](https://virt-tools.org)
+* [https://www.virttools.org](https://www.virttools.org)
+* [https://www.virt-tools.org](https://www.virt-tools.org)
+
+The site is setup to run under OpenShift
+
+Initial load can be done with
+
+```
+  oc process -f virttools-web/openshift/templates/virttools-web.json  | oc 
create -f -
+```
+
+Updates to the OpenShift config are manually activated using `oc replace`.
+
+Updates to the content itself are automatically propagated via a web hook.
-- 
2.24.1

[virttools-web PATCH 3/3] Remove obsolete openshift hosting configuration

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 openshift/templates/.gitignore |   2 -
 openshift/templates/update-tls.sh  |  16 --
 openshift/templates/virttools-web-tls.json |  82 ---
 openshift/templates/virttools-web.json | 272 -
 4 files changed, 372 deletions(-)
 delete mode 100644 openshift/templates/.gitignore
 delete mode 100755 openshift/templates/update-tls.sh
 delete mode 100644 openshift/templates/virttools-web-tls.json
 delete mode 100644 openshift/templates/virttools-web.json

diff --git a/openshift/templates/.gitignore b/openshift/templates/.gitignore
deleted file mode 100644
index 199c6e6..000
--- a/openshift/templates/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-tls-cert.pem
-tls-key.pem
diff --git a/openshift/templates/update-tls.sh 
b/openshift/templates/update-tls.sh
deleted file mode 100755
index 9b62697..000
--- a/openshift/templates/update-tls.sh
+++ /dev/null
@@ -1,16 +0,0 @@
-#!/bin/sh
-
-function die() {
-echo $*
-exit 1
-}
-
-test -f "tls-cert.pem" || die "Missing tls-cert.pem"
-test -f "tls-key.pem" || die "Missing tls-key.pem"
-
-TLS_CERT=`cat tls-cert.pem`
-TLS_KEY=`cat tls-key.pem`
-
-oc process -f virttools-web-tls.json | oc delete -f -
-oc process -p TLS_CERT="$TLS_CERT" -p TLS_KEY="$TLS_KEY" -f 
virttools-web-tls.json | oc create -f -
-
diff --git a/openshift/templates/virttools-web-tls.json 
b/openshift/templates/virttools-web-tls.json
deleted file mode 100644
index b7d1d33..000
--- a/openshift/templates/virttools-web-tls.json
+++ /dev/null
@@ -1,82 +0,0 @@
-{
-"apiVersion": "v1",
-"kind": "Template",
-"labels": {
-"app": "virttools-web",
-"template": "virttools-web"
-},
-"message": "The following service(s) have been created in your project: 
${NAME}.\n",
-"metadata": {
-"name": "virtttools-web",
-"annotations": {
-"openshift.io/display-name": "Virt Tools",
-"description": "Static web for virt-tools.org",
-"tags": "quickstart,httpd",
-"iconClass": "icon-apache",
-"openshift.io/long-description": "Template for virttools.org 
static web content",
-"openshift.io/provider-display-name": "Libvirt",
-"template.openshift.io/bindable": "false"
-}
-},
-"objects": [
-{
-"kind": "Route",
-"apiVersion": "v1",
-"metadata": {
-"name": "www.virt-tools.org"
-},
-"spec": {
-"host": "www.virt-tools.org",
-"to": {
-"kind": "Service",
-"name": "${NAME}"
-},
-   "tls": {
-   "termination": "edge",
-   "insecureEdgeTerminationPolicy": "Redirect",
-   "key": "${TLS_KEY}",
-   "certificate":  "${TLS_CERT}"
-   }
-}
-},
-{
-"kind": "Route",
-"apiVersion": "v1",
-"metadata": {
-"name": "www.virttools.org"
-},
-"spec": {
-"host": "www.virttools.org",
-"to": {
-"kind": "Service",
-"name": "${NAME}"
-},
-   "tls": {
-   "termination": "edge",
-   "insecureEdgeTerminationPolicy": "Redirect",
-   "key": "${TLS_KEY}",
-   "certificate":  "${TLS_CERT}"
-   }
-}
-}
-],
-"parameters": [
-{
-"name": "NAME",
-"displayName": "Name",
-"description": "The name assigned to all of the frontend objects 
defined in this template.",
-"required": true,
-"value": "virttools-web"
-},
-{
-"name": "TLS_KEY",
-"displayName": "TLS key for public routes",
-"description": "TLS key for public routes"
-},
-{
-"name": "TLS_CERT",
-"displayName": "TLS cert for public routes",
-"description": "TLS cert for public routes"
-}
-]
-}
diff --git a/openshift/templates/virttools-web.json 
b/openshift/templates/virttools-web.json
deleted file mode 100644
index 0e60a44..000
--- a/openshift/templates/virttools-web.json
+++ /dev/null
@@ -1,272 +0,0 @@
-{
-"apiVersion": "v1",
-"kind": "Template",
-"labels": {
-"app": "virttools-web",
-"template": "virttools-web"
-},
-"message": "The following service(s) have been created in your project: 
${NAME}.\n",
-"metadata": {
-"name": "virtttools-web",
-"annotations": {
-"openshift.io/display-name": "Virt Tools",
-"description": "Static web for virt-tools.org",
-"tags": "quickstart,httpd",
-"iconClass": "icon-apache",
-

[virttools-web PATCH 0/3] Switch www.virt-tools.org over to use GitLab Pages

[Daniel P . Berrangé]

This introduces use of GitLab CI + Pages to replace the current
OpenShift application which is only admin accessible by myself.
It also has automatic integration with LetsEncrypt guaranteeing
that we'll never have expired certificates.

Daniel P. Berrangé (3):
  Convert README to markdown format
  Introduce use of GitLab CI for publishing to GitLab Pages
  Remove obsolete openshift hosting configuration

 .gitlab-ci.yml |   9 +
 README |  19 --
 README.md  |  15 ++
 openshift/templates/.gitignore |   2 -
 openshift/templates/update-tls.sh  |  16 --
 openshift/templates/virttools-web-tls.json |  82 ---
 openshift/templates/virttools-web.json | 272 -
 {web => public}/header-bg.png  | Bin
 {web => public}/index.css  |   0
 {web => public}/index.html |   4 +-
 {web => public}/logo-kvm.png   | Bin
 {web => public}/logo-libguestfs.png| Bin
 {web => public}/logo-libosinfo.png | Bin
 {web => public}/logo-libvirt.png   | Bin
 {web => public}/logo-qemu.png  | Bin
 {web => public}/logo-virt-manager.png  | Bin
 {web => public}/logo.png   | Bin
 {web => public}/logo.xcf   | Bin
 18 files changed, 25 insertions(+), 394 deletions(-)
 create mode 100644 .gitlab-ci.yml
 delete mode 100644 README
 create mode 100644 README.md
 delete mode 100644 openshift/templates/.gitignore
 delete mode 100755 openshift/templates/update-tls.sh
 delete mode 100644 openshift/templates/virttools-web-tls.json
 delete mode 100644 openshift/templates/virttools-web.json
 rename {web => public}/header-bg.png (100%)
 rename {web => public}/index.css (100%)
 rename {web => public}/index.html (93%)
 rename {web => public}/logo-kvm.png (100%)
 rename {web => public}/logo-libguestfs.png (100%)
 rename {web => public}/logo-libosinfo.png (100%)
 rename {web => public}/logo-libvirt.png (100%)
 rename {web => public}/logo-qemu.png (100%)
 rename {web => public}/logo-virt-manager.png (100%)
 rename {web => public}/logo.png (100%)
 rename {web => public}/logo.xcf (100%)

-- 
2.24.1

[virttools-planet PATCH 4/4] Drop Amit Shah and Nathan Gauër

[Daniel P . Berrangé]

Both these feeds are returning errors due to moved URLs. In addition the
sites have not been updated with any virtualization related content for
several years.

Signed-off-by: Daniel P. Berrangé 
---
 virt-tools/config.ini | 6 --
 1 file changed, 6 deletions(-)

diff --git a/virt-tools/config.ini b/virt-tools/config.ini
index eff9a20..8ff5423 100644
--- a/virt-tools/config.ini
+++ b/virt-tools/config.ini
@@ -114,9 +114,6 @@ faceheight = 80
 [https://vfio.blogspot.com/feeds/posts/default]
 name = Alex Williamson
 
-[https://log.amitshah.net/feed/]
-name = Amit Shah
-
 [https://www.kraxel.org/blog/feed/]
 name = Gerd Hoffmann
 
@@ -162,9 +159,6 @@ faceheight = 80
 [http://bosdonnat.fr/feeds/virtualization.atom.xml]
 name = Cédric Bosdonnat
 
-[http://www.studiopixl.com/blog/index.php/feeds/rss]
-name = Nathan Gauër
-
 [https://virtualpenguins.blogspot.com/feeds/posts/default]
 name = Cornelia Huck
 
-- 
2.24.1

[virttools-planet PATCH 3/4] Remove obsolete openshift hosting configuration

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 openshift/templates/.gitignore|   2 -
 openshift/templates/update-tls.sh |  16 -
 openshift/templates/virttools-planet-tls.json |  82 
 openshift/templates/virttools-planet.json | 425 --
 web/httpd-cfg/cors.conf   |   3 -
 5 files changed, 528 deletions(-)
 delete mode 100644 openshift/templates/.gitignore
 delete mode 100755 openshift/templates/update-tls.sh
 delete mode 100644 openshift/templates/virttools-planet-tls.json
 delete mode 100644 openshift/templates/virttools-planet.json
 delete mode 100644 web/httpd-cfg/cors.conf

diff --git a/openshift/templates/.gitignore b/openshift/templates/.gitignore
deleted file mode 100644
index 199c6e6..000
--- a/openshift/templates/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-tls-cert.pem
-tls-key.pem
diff --git a/openshift/templates/update-tls.sh 
b/openshift/templates/update-tls.sh
deleted file mode 100755
index 0351959..000
--- a/openshift/templates/update-tls.sh
+++ /dev/null
@@ -1,16 +0,0 @@
-#!/bin/sh
-
-function die() {
-echo $*
-exit 1
-}
-
-test -f "tls-cert.pem" || die "Missing tls-cert.pem"
-test -f "tls-key.pem" || die "Missing tls-key.pem"
-
-TLS_CERT=`cat tls-cert.pem`
-TLS_KEY=`cat tls-key.pem`
-
-oc process -f virttools-planet-tls.json | oc delete -f -
-oc process -p TLS_CERT="$TLS_CERT" -p TLS_KEY="$TLS_KEY" -f 
virttools-planet-tls.json | oc create -f -
-
diff --git a/openshift/templates/virttools-planet-tls.json 
b/openshift/templates/virttools-planet-tls.json
deleted file mode 100644
index 1f26f2e..000
--- a/openshift/templates/virttools-planet-tls.json
+++ /dev/null
@@ -1,82 +0,0 @@
-{
-"apiVersion": "v1",
-"kind": "Template",
-"labels": {
-"app": "virttools-planet",
-"template": "virttools-planet"
-},
-"message": "The following service(s) have been created in your project: 
${NAME}.\n",
-"metadata": {
-"name": "virtttools-planet",
-"annotations": {
-"openshift.io/display-name": "Virt Tools",
-"description": "Static web for planet.virt-tools.org",
-"tags": "quickstart,httpd",
-"iconClass": "icon-apache",
-"openshift.io/long-description": "Template for 
planet.virttools.org static web content",
-"openshift.io/provider-display-name": "Libvirt",
-"template.openshift.io/bindable": "false"
-}
-},
-"objects": [
-{
-"kind": "Route",
-"apiVersion": "v1",
-"metadata": {
-"name": "planet.virt-tools.org"
-},
-"spec": {
-"host": "planet.virt-tools.org",
-"to": {
-"kind": "Service",
-"name": "${NAME}"
-},
-   "tls": {
-   "termination": "edge",
-   "insecureEdgeTerminationPolicy": "Redirect",
-   "key": "${TLS_KEY}",
-   "certificate":  "${TLS_CERT}"
-   }
-}
-},
-{
-"kind": "Route",
-"apiVersion": "v1",
-"metadata": {
-"name": "planet.virttools.org"
-},
-"spec": {
-"host": "planet.virttools.org",
-"to": {
-"kind": "Service",
-"name": "${NAME}"
-},
-   "tls": {
-   "termination": "edge",
-   "insecureEdgeTerminationPolicy": "Redirect",
-   "key": "${TLS_KEY}",
-   "certificate":  "${TLS_CERT}"
-   }
-}
-}
-],
-"parameters": [
-{
-"name": "NAME",
-"displayName": "Name",
-"description": "The name assigned to all of the frontend objects 
defined in this template.",
-"required": true,
-"value": "virttools-planet"
-},
-{
-"name": "TLS_KEY",
-"displayName": "TLS key for public routes",
-"description": "TLS key for public routes"
-},
-{
-"name": "TLS_CERT",
-"displayName": "TLS cert for public routes",
-"description": "TLS cert for public routes"
-}
-]
-}
diff --git a/openshift/templates/virttools-planet.json 
b/openshift/templates/virttools-planet.json
deleted file mode 100644
index 749f180..000
--- a/openshift/templates/virttools-planet.json
+++ /dev/null
@@ -1,425 +0,0 @@
-{
-"apiVersion": "v1",
-"kind": "Template",
-"labels": {
-"app": "virttools-planet",
-"template": "virttools-planet"
-},
-"message": "The following service(s) have been created in your project: 
${NAME}.\n",
-"metadata": {
-"name": "virtttools-planet",
-"annotations": {
-"openshift.io/display-name": 

[virttools-planet PATCH 0/4] Switch planet.virt-tools.org over to use GitLab Pages

[Daniel P . Berrangé]

This introduces use of GitLab CI + Pages to replace the current
OpenShift application which is only admin accessible by myself.
It also has automatic integration with LetsEncrypt guaranteeing
that we'll never have expired certificates.

Daniel P. Berrangé (4):
  Convert README to markdown format
  Introduce use of GitLab CI for publishing to GitLab Pages
  Remove obsolete openshift hosting configuration
  Drop Amit Shah and Nathan Gauër

 .gitlab-ci.yml|  14 +
 README|  39 --
 README.md |  43 ++
 openshift/templates/.gitignore|   2 -
 openshift/templates/update-tls.sh |  16 -
 openshift/templates/virttools-planet-tls.json |  82 
 openshift/templates/virttools-planet.json | 425 --
 updater/planet-cache.py => planet-cache.py|   0
 updater/planet.py => planet.py|   0
 {updater/planet => planet}/__init__.py|   0
 {updater/planet => planet}/atomstyler.py  |   0
 {updater/planet => planet}/cache.py   |   0
 .../compat_logging/__init__.py|   0
 .../compat_logging/config.py  |   0
 .../compat_logging/handlers.py|   0
 {updater/planet => planet}/feedparser.py  |   0
 {updater/planet => planet}/htmltmpl.py|   0
 {updater/planet => planet}/sanitize.py|   0
 {updater/planet => planet}/tests/__init__.py  |   0
 .../planet => planet}/tests/test_channel.py   |   0
 {updater/planet => planet}/tests/test_main.py |   0
 .../planet => planet}/tests/test_sanitize.py  |   0
 {updater/planet => planet}/timeoutsocket.py   |   0
 .../images/alexbennee.jpeg| Bin
 .../virt-tools => public}/images/berrange.png | Bin
 .../virt-tools => public}/images/cole.png | Bin
 .../images/ehabkost.jpeg  | Bin
 .../images/header-bg.png  | Bin
 .../virt-tools => public}/images/kashyap.jpeg | Bin
 .../virt-tools => public}/images/logo.png | Bin
 .../virt-tools => public}/images/logo.xcf | Bin
 .../virt-tools => public}/images/otubo.png| Bin
 .../virt-tools => public}/images/qemu.png | Bin
 .../virt-tools => public}/images/rjones.jpeg  | Bin
 .../virt-tools => public}/images/sgarzare.png | Bin
 .../virt-tools => public}/images/teuf.png | Bin
 .../images/thomashuth.png | Bin
 .../virt-tools => public}/images/ybettan.png  | Bin
 .../virt-tools => public}/images/zeenix.png   | Bin
 updater/app.py|  18 -
 .../virt-tools => virt-tools}/atom.xml.tmpl   |   0
 .../basic/index.html.tmpl |   7 +-
 .../basic/style.css.tmpl  |   0
 {updater/virt-tools => virt-tools}/config.ini |  10 +-
 .../foafroll.xml.tmpl |   0
 .../virt-tools => virt-tools}/opml.xml.tmpl   |   0
 .../virt-tools => virt-tools}/rss10.xml.tmpl  |   0
 .../virt-tools => virt-tools}/rss20.xml.tmpl  |   0
 web/httpd-cfg/cors.conf   |   3 -
 49 files changed, 62 insertions(+), 597 deletions(-)
 create mode 100644 .gitlab-ci.yml
 delete mode 100644 README
 create mode 100644 README.md
 delete mode 100644 openshift/templates/.gitignore
 delete mode 100755 openshift/templates/update-tls.sh
 delete mode 100644 openshift/templates/virttools-planet-tls.json
 delete mode 100644 openshift/templates/virttools-planet.json
 rename updater/planet-cache.py => planet-cache.py (100%)
 rename updater/planet.py => planet.py (100%)
 rename {updater/planet => planet}/__init__.py (100%)
 rename {updater/planet => planet}/atomstyler.py (100%)
 rename {updater/planet => planet}/cache.py (100%)
 rename {updater/planet => planet}/compat_logging/__init__.py (100%)
 rename {updater/planet => planet}/compat_logging/config.py (100%)
 rename {updater/planet => planet}/compat_logging/handlers.py (100%)
 rename {updater/planet => planet}/feedparser.py (100%)
 rename {updater/planet => planet}/htmltmpl.py (100%)
 rename {updater/planet => planet}/sanitize.py (100%)
 rename {updater/planet => planet}/tests/__init__.py (100%)
 rename {updater/planet => planet}/tests/test_channel.py (100%)
 rename {updater/planet => planet}/tests/test_main.py (100%)
 rename {updater/planet => planet}/tests/test_sanitize.py (100%)
 rename {updater/planet => planet}/timeoutsocket.py (100%)
 rename {updater/virt-tools => public}/images/alexbennee.jpeg (100%)
 rename {updater/virt-tools => public}/images/berrange.png (100%)
 rename {updater/virt-tools => public}/images/cole.png (100%)
 rename {updater/virt-tools => public}/images/ehabkost.jpeg (100%)
 rename {updater/virt-tools => public}/images/header-bg.png (100%)
 rename {updater/virt-tools => public}/images/kashyap.jpeg (100%)
 rename {updater/virt-tools => public}/images/logo.png (100%)
 rename {updater/virt-tools => public}/images/logo.xcf (100%)
 rename {updater/virt-tools => public}/images/otubo.png (100%)
 rename 

[virttools-planet PATCH 2/4] Introduce use of GitLab CI for publishing to GitLab Pages

[Daniel P . Berrangé]

To publish to GitLab pages, all that is required is to provide a CI job
that creates files in a directory called "public" and list that as an
artifact.

When the CI job completes, the website is immediately available at the
URL  https://username.gitlab.io/reponame. This makes it much easier to
preview changes to the site than with OpenShift apps.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.yml|  14 
 README.md |  20 +++---
 updater/planet-cache.py => planet-cache.py|   0
 updater/planet.py => planet.py|   0
 {updater/planet => planet}/__init__.py|   0
 {updater/planet => planet}/atomstyler.py  |   0
 {updater/planet => planet}/cache.py   |   0
 .../compat_logging/__init__.py|   0
 .../compat_logging/config.py  |   0
 .../compat_logging/handlers.py|   0
 {updater/planet => planet}/feedparser.py  |   0
 {updater/planet => planet}/htmltmpl.py|   0
 {updater/planet => planet}/sanitize.py|   0
 {updater/planet => planet}/tests/__init__.py  |   0
 .../planet => planet}/tests/test_channel.py   |   0
 {updater/planet => planet}/tests/test_main.py |   0
 .../planet => planet}/tests/test_sanitize.py  |   0
 {updater/planet => planet}/timeoutsocket.py   |   0
 .../images/alexbennee.jpeg| Bin
 .../virt-tools => public}/images/berrange.png | Bin
 .../virt-tools => public}/images/cole.png | Bin
 .../images/ehabkost.jpeg  | Bin
 .../images/header-bg.png  | Bin
 .../virt-tools => public}/images/kashyap.jpeg | Bin
 .../virt-tools => public}/images/logo.png | Bin
 .../virt-tools => public}/images/logo.xcf | Bin
 .../virt-tools => public}/images/otubo.png| Bin
 .../virt-tools => public}/images/qemu.png | Bin
 .../virt-tools => public}/images/rjones.jpeg  | Bin
 .../virt-tools => public}/images/sgarzare.png | Bin
 .../virt-tools => public}/images/teuf.png | Bin
 .../images/thomashuth.png | Bin
 .../virt-tools => public}/images/ybettan.png  | Bin
 .../virt-tools => public}/images/zeenix.png   | Bin
 updater/app.py|  18 
 .../virt-tools => virt-tools}/atom.xml.tmpl   |   0
 .../basic/index.html.tmpl |   7 +++---
 .../basic/style.css.tmpl  |   0
 {updater/virt-tools => virt-tools}/config.ini |   4 ++--
 .../foafroll.xml.tmpl |   0
 .../virt-tools => virt-tools}/opml.xml.tmpl   |   0
 .../virt-tools => virt-tools}/rss10.xml.tmpl  |   0
 .../virt-tools => virt-tools}/rss20.xml.tmpl  |   0
 43 files changed, 27 insertions(+), 36 deletions(-)
 create mode 100644 .gitlab-ci.yml
 rename updater/planet-cache.py => planet-cache.py (100%)
 rename updater/planet.py => planet.py (100%)
 rename {updater/planet => planet}/__init__.py (100%)
 rename {updater/planet => planet}/atomstyler.py (100%)
 rename {updater/planet => planet}/cache.py (100%)
 rename {updater/planet => planet}/compat_logging/__init__.py (100%)
 rename {updater/planet => planet}/compat_logging/config.py (100%)
 rename {updater/planet => planet}/compat_logging/handlers.py (100%)
 rename {updater/planet => planet}/feedparser.py (100%)
 rename {updater/planet => planet}/htmltmpl.py (100%)
 rename {updater/planet => planet}/sanitize.py (100%)
 rename {updater/planet => planet}/tests/__init__.py (100%)
 rename {updater/planet => planet}/tests/test_channel.py (100%)
 rename {updater/planet => planet}/tests/test_main.py (100%)
 rename {updater/planet => planet}/tests/test_sanitize.py (100%)
 rename {updater/planet => planet}/timeoutsocket.py (100%)
 rename {updater/virt-tools => public}/images/alexbennee.jpeg (100%)
 rename {updater/virt-tools => public}/images/berrange.png (100%)
 rename {updater/virt-tools => public}/images/cole.png (100%)
 rename {updater/virt-tools => public}/images/ehabkost.jpeg (100%)
 rename {updater/virt-tools => public}/images/header-bg.png (100%)
 rename {updater/virt-tools => public}/images/kashyap.jpeg (100%)
 rename {updater/virt-tools => public}/images/logo.png (100%)
 rename {updater/virt-tools => public}/images/logo.xcf (100%)
 rename {updater/virt-tools => public}/images/otubo.png (100%)
 rename {updater/virt-tools => public}/images/qemu.png (100%)
 rename {updater/virt-tools => public}/images/rjones.jpeg (100%)
 rename {updater/virt-tools => public}/images/sgarzare.png (100%)
 rename {updater/virt-tools => public}/images/teuf.png (100%)
 rename {updater/virt-tools => public}/images/thomashuth.png (100%)
 rename {updater/virt-tools => public}/images/ybettan.png (100%)
 rename {updater/virt-tools => public}/images/zeenix.png (100%)
 delete mode 100755 updater/app.py
 rename {updater/virt-tools => virt-tools}/atom.xml.tmpl (100%)
 rename {updater/virt-tools => virt-tools}/basic/index.html.tmpl (91%)
 rename {updater/virt-tools => 

[virttools-planet PATCH 1/4] Convert README to markdown format

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 README| 39 ---
 README.md | 47 +++
 2 files changed, 47 insertions(+), 39 deletions(-)
 delete mode 100644 README
 create mode 100644 README.md

diff --git a/README b/README
deleted file mode 100644
index ad388f8..000
--- a/README
+++ /dev/null
@@ -1,39 +0,0 @@
-  Virt Tools planet site
-  ==
-
-This directory contains content / configuration for managing
-
-http(s)://planet.virttools.org
-http(s)://planet.virt-tools.org
-
-How to add your blog
-
-Add a config.ini section for your blog to updater/virt-tools/config.ini:
-
-  [https://example.org/my/blog/feed/]
-  name = John Doe
-  face = jdoe.png
-  facewidth = 96
-  faceheight = 96
-
-Where face (logo image filename), facewidth (logo image width in pixels), and
-faceheight (logo image height in pixels) are optional attributes that describe
-the logo image associated with your blog.  Remember to add your image file into
-the updater/virt-tools/images/ directory if you wish to use an image.
-
-Please send a patch email to libvir-list@redhat.com:
-
-  $ git commit -as
-  $ git send-email --to libvir-list@redhat.com --cc berra...@redhat.com HEAD^..
-
-How to run the site

-The site is setup to run under OpenShift
-
-Initial load can be done with
-
-  oc process -f openshift/templates/virttools-planet.json  | oc create -f -
-
-Updates to the OpenShift config are manually activated using 'oc replace'.
-
-Updates to the content itself are automatically propagated via a planet hook.
diff --git a/README.md b/README.md
new file mode 100644
index 000..68ae162
--- /dev/null
+++ b/README.md
@@ -0,0 +1,47 @@
+Virt Tools planet site
+==
+
+This directory contains content / configuration for managing
+
+* [http://planet.virttools.org](http://planet.virttools.org)
+* [http://planet.virt-tools.org](http://planet.virt-tools.org)
+
+How to add your blog
+
+Add a config.ini section for your blog to `updater/virt-tools/config.ini`:
+
+```
+  [https://example.org/my/blog/feed/]
+  name = John Doe
+  face = jdoe.png
+  facewidth = 96
+  faceheight = 96
+```
+
+Where `face` (logo image filename), `facewidth` (logo image width in pixels),
+and `faceheight` (logo image height in pixels) are optional attributes that
+describe the logo image associated with your blog.  Remember to add your image
+file into the `updater/virt-tools/images/` directory if you wish to use an
+image.
+
+Please send a patch email to `libvir-list@redhat.com`:
+
+```
+  $ git commit -as
+  $ git send-email --to libvir-list@redhat.com --cc berra...@redhat.com HEAD^..
+```
+
+How to run the site
+---
+
+The site is setup to run under OpenShift
+
+Initial load can be done with
+
+```
+  oc process -f openshift/templates/virttools-planet.json  | oc create -f -
+```
+
+Updates to the OpenShift config are manually activated using `oc replace`.
+
+Updates to the content itself are automatically propagated via a planet hook.
-- 
2.24.1

Re: [libvirt PATCH 3/3] qemu: block: external snapshot-delete implementation for straightforward cases

[Peter Krempa]

On Tue, Mar 31, 2020 at 11:18:29 +0200, Pavel Mores wrote:
> Works by blockcommitting the snapshot to be deleted into its parent.  Handles
> both deleting children as well and deleting children only (commits the
> children into the snapshot referred to by snapshot-delete argument).  If the
> necessary block commit operation turns out to be active (the snapshot referred
> to by snapshot-delete argument is current, or deleting children is requested)
> pivot is performed as well.
> 
> This implemetation is limited to the straightforward case which assumes no
> branching snapshot chains below the snapshot to be deleted, and the current
> snapshot has to be the leaf of the (linear) chain starting at the snapshot to
> be deleted.  These requirements are checked and enforced at the top of
> qemuDomainSnapshotDeleteExternal().  They might even be too restrictive for
> the case where deletion of children is not requested as in that case,
> requiring that the parent of the snapshot to be deleted not be a branching
> point in snapshot tree should be enough.
> 
> The above limitations do not appear to be too severe under the current state
> of matters where a major source of branching snapshot structures,
> snapshot-revert, is not even implemented for external snapshots yet.  The only
> other known cause of branching in external snapshots thus seems to be if a
> user creates branching by manually creating snapshot images and metadata and
> applies them using snapshot-create --redefine.  At any rate, this work should
> be understood as just a first step to a full support of deleting external
> snapshots.
> 
> Signed-off-by: Pavel Mores 
> ---
>  src/qemu/qemu_driver.c | 149 +++--
>  1 file changed, 145 insertions(+), 4 deletions(-)

Few quick comments:

> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b981745ecf..4d4f3f069f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -16707,6 +16707,149 @@ qemuDomainMomentReparentChildren(void *payload,
>  }
>  
>  
> +/*
> + * We can't use virDomainMomentFindLeaf() as it takes a
> + * virDomainMomentObjListPtr which we don't have.  It might be a good idea to
> + * move this function to a library of virDomainMomentObj helpers, then
> + * reimplement virDomainMomentFindLeaf() in terms of this function as it only
> + * uses its virDomainMomentObjListPtr parameter to fish a 
> virDomainMomentObjPtr
> + * out of it.  Then it just runs this function's algorithm on it.

Please do so ...

> + */
> +static virDomainMomentObjPtr
> +myDomainMomentFindLeaf(virDomainMomentObjPtr moment)

... since this does not conform to the naming scheme.

> +{
> +if (moment->nchildren != 1)
> +return NULL;
> +while (moment->nchildren == 1)
> +moment = moment->first_child;
> +if (moment->nchildren == 0)
> +return moment;
> +return NULL;
> +}
> +
> +
> +static int
> +qemuDomainSnapshotDeleteExternal(virDomainObjPtr vm,
> + virQEMUDriverPtr driver,
> + virDomainMomentObjPtr snap,
> + unsigned int flags)
> +{
> +int ret = -1;
> +bool isActive;
> +virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap);
> +virDomainMomentObjPtr leaf = snap->nchildren ? 
> myDomainMomentFindLeaf(snap) : snap;

Please refrain from using ternary operators here.

> +virDomainMomentObjPtr parent = snap->parent;
> +virDomainSnapshotDefPtr parentdef = virDomainSnapshotObjGetDef(parent);
> +size_t i;
> +
> +/* This function only works if the chain below 'snap' is linear.  If
> + * there's no unique leaf it means the chain of 'snap's children
> + * branches at some point.  Also, if there *is* a leaf but it's not
> + * the current snapshot, bail out as well. */
> +if (leaf == NULL) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +   "%s", _("can't delete, snapshot chain branches"));
> +goto cleanup;
> +}
> +if (leaf != virDomainSnapshotGetCurrent(vm->snapshots)) {

Btw, vm->snapshots.base is the 'virDomainMomentObjListPtr' you were
looking for above.

> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +   "%s", _("can't delete, leaf snapshot is not 
> current"));
> +goto cleanup;
> +}

Check that VM is active should be added here, so that we can bail out
sooner than when we try to actually commit.

> +
> +if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
> + VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
> +isActive = true;
> +} else {
> +isActive = snap == virDomainSnapshotGetCurrent(vm->snapshots);
> +}
> +
> +for (i = 0; i < snapdef->ndisks; i++) {
> +virDomainSnapshotDiskDefPtr snapdisk = &(snapdef->disks[i]);
> +const char *diskName = snapdisk->name;
> +const char *basePath = NULL;
> +const 

Re: [GSoC][RPC] Project Proposal: "Introducing Job control to the storage driver"

[Michal Prívozník]

On 31. 3. 2020 2:27, Prathamesh Chavan wrote:
> GSoC Proposal - 2020
> 

Looks good. It's visible that you've already participated in GSoC
previously. Just don't forget to make it final. The deadline is in ~8 hours.

Michal

[libvirt PATCH 3/3] qemu: block: external snapshot-delete implementation for straightforward cases

[Pavel Mores]

Works by blockcommitting the snapshot to be deleted into its parent.  Handles
both deleting children as well and deleting children only (commits the
children into the snapshot referred to by snapshot-delete argument).  If the
necessary block commit operation turns out to be active (the snapshot referred
to by snapshot-delete argument is current, or deleting children is requested)
pivot is performed as well.

This implemetation is limited to the straightforward case which assumes no
branching snapshot chains below the snapshot to be deleted, and the current
snapshot has to be the leaf of the (linear) chain starting at the snapshot to
be deleted.  These requirements are checked and enforced at the top of
qemuDomainSnapshotDeleteExternal().  They might even be too restrictive for
the case where deletion of children is not requested as in that case,
requiring that the parent of the snapshot to be deleted not be a branching
point in snapshot tree should be enough.

The above limitations do not appear to be too severe under the current state
of matters where a major source of branching snapshot structures,
snapshot-revert, is not even implemented for external snapshots yet.  The only
other known cause of branching in external snapshots thus seems to be if a
user creates branching by manually creating snapshot images and metadata and
applies them using snapshot-create --redefine.  At any rate, this work should
be understood as just a first step to a full support of deleting external
snapshots.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_driver.c | 149 +++--
 1 file changed, 145 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b981745ecf..4d4f3f069f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16707,6 +16707,149 @@ qemuDomainMomentReparentChildren(void *payload,
 }
 
 
+/*
+ * We can't use virDomainMomentFindLeaf() as it takes a
+ * virDomainMomentObjListPtr which we don't have.  It might be a good idea to
+ * move this function to a library of virDomainMomentObj helpers, then
+ * reimplement virDomainMomentFindLeaf() in terms of this function as it only
+ * uses its virDomainMomentObjListPtr parameter to fish a virDomainMomentObjPtr
+ * out of it.  Then it just runs this function's algorithm on it.
+ */
+static virDomainMomentObjPtr
+myDomainMomentFindLeaf(virDomainMomentObjPtr moment)
+{
+if (moment->nchildren != 1)
+return NULL;
+while (moment->nchildren == 1)
+moment = moment->first_child;
+if (moment->nchildren == 0)
+return moment;
+return NULL;
+}
+
+
+static int
+qemuDomainSnapshotDeleteExternal(virDomainObjPtr vm,
+ virQEMUDriverPtr driver,
+ virDomainMomentObjPtr snap,
+ unsigned int flags)
+{
+int ret = -1;
+bool isActive;
+virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap);
+virDomainMomentObjPtr leaf = snap->nchildren ? 
myDomainMomentFindLeaf(snap) : snap;
+virDomainMomentObjPtr parent = snap->parent;
+virDomainSnapshotDefPtr parentdef = virDomainSnapshotObjGetDef(parent);
+size_t i;
+
+/* This function only works if the chain below 'snap' is linear.  If
+ * there's no unique leaf it means the chain of 'snap's children
+ * branches at some point.  Also, if there *is* a leaf but it's not
+ * the current snapshot, bail out as well. */
+if (leaf == NULL) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   "%s", _("can't delete, snapshot chain branches"));
+goto cleanup;
+}
+if (leaf != virDomainSnapshotGetCurrent(vm->snapshots)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   "%s", _("can't delete, leaf snapshot is not current"));
+goto cleanup;
+}
+
+if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
+ VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
+isActive = true;
+} else {
+isActive = snap == virDomainSnapshotGetCurrent(vm->snapshots);
+}
+
+for (i = 0; i < snapdef->ndisks; i++) {
+virDomainSnapshotDiskDefPtr snapdisk = &(snapdef->disks[i]);
+const char *diskName = snapdisk->name;
+const char *basePath = NULL;
+const char *topPath = snapdisk->src->path;
+int blockCommitFlags = VIR_DOMAIN_BLOCK_COMMIT_DELETE;
+virDomainDiskDefPtr disk;
+
+if (!(disk = qemuDomainDiskByName(vm->def, diskName)))
+goto cleanup;
+
+if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
+basePath = snapdisk->src->path;
+topPath = disk->src->path;
+} else {
+if (parent->nchildren == 1) {
+if (parentdef == NULL) {
+virStorageSourcePtr baseSrc = 
virStorageFileChainLookup(disk->src, NULL, NULL, 0, NULL);
+if 

[libvirt PATCH 0/3] qemu: block: basic implementation of deletion of external snapshots

[Pavel Mores]

Deleting external snapshots has been unimplemented so far.  This series aims
to enable limited functionality in that direction, handling just the common
cases for now.  The intention is to subsequently build upon this to eventually
cover the more complex/exotic cases as well.

Please refer to the commit message of the final commit for a detailed
description of features and limitations of this change.

Pavel Mores (3):
  qemu: block: factor implementation out of qemuDomainBlockCommit()
  qemu: block: factor implementation out of qemuDomainBlockJobAbort()
  qemu: block: external snapshot-delete implementation for
straightforward cases

 src/qemu/qemu_driver.c | 295 +
 1 file changed, 241 insertions(+), 54 deletions(-)

-- 
2.24.1

[libvirt PATCH 2/3] qemu: block: factor implementation out of qemuDomainBlockJobAbort()

[Pavel Mores]

As with the previous commit, this should primarily make
qemuDomainBlockJobAbort() callable from within the QEMU driver.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_driver.c | 62 --
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f94636f651..b981745ecf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -163,6 +163,12 @@ qemuDomainBlockCommitImpl(virDomainObjPtr vm,
   unsigned long bandwidth,
   unsigned int flags);
 
+static int
+qemuDomainBlockJobAbortImpl(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+const char *path,
+unsigned int flags);
+
 /* Looks up the domain object from snapshot and unlocks the
  * driver. The returned domain object is locked and ref'd and the
  * caller must call virDomainObjEndAPI() on it. */
@@ -17440,45 +17446,31 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
 return ret;
 }
 
-
 static int
-qemuDomainBlockJobAbort(virDomainPtr dom,
-const char *path,
-unsigned int flags)
+qemuDomainBlockJobAbortImpl(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+const char *path,
+unsigned int flags)
 {
-virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainDiskDefPtr disk = NULL;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT);
 bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC);
 g_autoptr(qemuBlockJobData) job = NULL;
-virDomainObjPtr vm;
 qemuDomainObjPrivatePtr priv = NULL;
 bool blockdev = false;
 int ret = -1;
 
-virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC |
-  VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, -1);
-
-if (!(vm = qemuDomainObjFromDomain(dom)))
-return -1;
-
-if (virDomainBlockJobAbortEnsureACL(dom->conn, vm->def) < 0)
-goto cleanup;
-
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
-goto cleanup;
-
 if (virDomainObjCheckActive(vm) < 0)
-goto endjob;
+return -1;
 
 if (!(disk = qemuDomainDiskByName(vm->def, path)))
-goto endjob;
+return -1;
 
 if (!(job = qemuBlockJobDiskGetJob(disk))) {
 virReportError(VIR_ERR_INVALID_ARG,
_("disk %s does not have an active block job"), 
disk->dst);
-goto endjob;
+return -1;
 }
 
 priv = vm->privateData;
@@ -17549,6 +17541,34 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
  endjob:
 if (job && !async)
 qemuBlockJobSyncEnd(vm, job, QEMU_ASYNC_JOB_NONE);
+
+return ret;
+}
+
+
+static int
+qemuDomainBlockJobAbort(virDomainPtr dom,
+const char *path,
+unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC |
+  VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, -1);
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+return -1;
+
+if (virDomainBlockJobAbortEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
+ret = qemuDomainBlockJobAbortImpl(driver, vm, path, flags);
+
 qemuDomainObjEndJob(driver, vm);
 
  cleanup:
-- 
2.24.1

[libvirt PATCH 1/3] qemu: block: factor implementation out of qemuDomainBlockCommit()

[Pavel Mores]

The idea is to make block commit callable from within the QEMU driver.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_driver.c | 84 +++---
 1 file changed, 55 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9a62684f0..f94636f651 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -154,6 +154,15 @@ static int qemuOpenFileAs(uid_t fallback_uid, gid_t 
fallback_gid,
 
 static virQEMUDriverPtr qemu_driver;
 
+static int
+qemuDomainBlockCommitImpl(virDomainObjPtr vm,
+  virQEMUDriverPtr driver,
+  const char *path,
+  const char *base,
+  const char *top,
+  unsigned long bandwidth,
+  unsigned int flags);
+
 /* Looks up the domain object from snapshot and unlocks the
  * driver. The returned domain object is locked and ref'd and the
  * caller must call virDomainObjEndAPI() on it. */
@@ -18314,18 +18323,16 @@ qemuDomainBlockPull(virDomainPtr dom, const char 
*path, unsigned long bandwidth,
 return qemuDomainBlockPullCommon(vm, path, NULL, bandwidth, flags);
 }
 
-
 static int
-qemuDomainBlockCommit(virDomainPtr dom,
-  const char *path,
-  const char *base,
-  const char *top,
-  unsigned long bandwidth,
-  unsigned int flags)
+qemuDomainBlockCommitImpl(virDomainObjPtr vm,
+  virQEMUDriverPtr driver,
+  const char *path,
+  const char *base,
+  const char *top,
+  unsigned long bandwidth,
+  unsigned int flags)
 {
-virQEMUDriverPtr driver = dom->conn->privateData;
-qemuDomainObjPrivatePtr priv;
-virDomainObjPtr vm = NULL;
+qemuDomainObjPrivatePtr priv = vm->privateData;
 const char *device = NULL;
 const char *jobname = NULL;
 int ret = -1;
@@ -18347,25 +18354,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
 bool persistjob = false;
 bool blockdev = false;
 
-virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
-  VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
-  VIR_DOMAIN_BLOCK_COMMIT_RELATIVE |
-  VIR_DOMAIN_BLOCK_COMMIT_DELETE |
-  VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1);
-
-if (!(vm = qemuDomainObjFromDomain(dom)))
-goto cleanup;
-priv = vm->privateData;
-
-if (virDomainBlockCommitEnsureACL(dom->conn, vm->def) < 0)
-goto cleanup;
-
-if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
-goto cleanup;
-
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
-goto cleanup;
-
 if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
 
@@ -18558,6 +18546,43 @@ qemuDomainBlockCommit(virDomainPtr dom,
 virErrorRestore(_err);
 }
 qemuBlockJobStartupFinalize(vm, job);
+
+return ret;
+}
+
+
+static int
+qemuDomainBlockCommit(virDomainPtr dom,
+  const char *path,
+  const char *base,
+  const char *top,
+  unsigned long bandwidth,
+  unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+  VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
+  VIR_DOMAIN_BLOCK_COMMIT_RELATIVE |
+  VIR_DOMAIN_BLOCK_COMMIT_DELETE |
+  VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1);
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainBlockCommitEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
+ret = qemuDomainBlockCommitImpl(vm, driver, path, base, top, bandwidth, 
flags);
+
 qemuDomainObjEndJob(driver, vm);
 
  cleanup:
@@ -18565,6 +18590,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
 return ret;
 }
 
+
 static int
 qemuDomainOpenGraphics(virDomainPtr dom,
unsigned int idx,
-- 
2.24.1

[PATCH] Add blog for ARM Datacenter Project.

[Robert Foley]

Our blog covers QEMU topics.

Signed-off-by: Robert Foley 
---
 updater/virt-tools/config.ini | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/updater/virt-tools/config.ini b/updater/virt-tools/config.ini
index 9375c82..4e53029 100644
--- a/updater/virt-tools/config.ini
+++ b/updater/virt-tools/config.ini
@@ -197,3 +197,6 @@ faceheight = 96
 
 [https://fidencio.org/tags/virt/index.xml]
 name = Fabiano Fidêncio
+
+[https://futurewei-cloud.github.io/ARM-Datacenter/feed.xml]
+name = ARM Datacenter Project
-- 
2.17.1

Re: [PATCH] Add blog for ARM Datacenter Project.

[Daniel P . Berrangé]

On Mon, Mar 30, 2020 at 02:39:36PM -0400, Robert Foley wrote:
> Our blog covers QEMU topics.
> 
> Signed-off-by: Robert Foley 
> ---
>  updater/virt-tools/config.ini | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Daniel P. Berrangé 

It is pushed and will be live within the next hour


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

Re: GSoC'20 Interested Student: Adding support to Jailhouse Hypervisor

[Jan Kiszka]

On 31.03.20 04:43, PRAKHAR BANSAL wrote:

Hi Jan,

Thanks for the confirmation to proceed on project proposal.

Also, I tried installing Jailhouse on my VM after enabling VT-x/EPT and
IOMMU for my VM(Guest OS- Ubuntu 18.04) on VMware fusion hypervisor with
MacOS on the host side.
However,  Jailhouse hardware check was failed because of missing
*Preemption timer and Virtualize APIC access*, could you please suggest,
if this is hardware limitation?  Is there any workaround here?


You will need a hypervisor that supports both when nesting, but I have
no idea if there is one for the Mac. What is known to work is KVM on
Linux hosts.


My laptop's processor is Intel quad-core i7.

image.png

Also, could you please suggest, if I can talk to you through an IRC or
slack channel since it is a bit hard to communicate over email every time.


I'll be listening on #jailhouse, irc.freenode.net.

Jan



Thanks,
Prakhar


On Mon, Mar 30, 2020 at 6:15 AM Jan Kiszka mailto:jan.kis...@web.de>> wrote:

On 30.03.20 10:02, PRAKHAR BANSAL wrote:
 > Hi Jan,
 >
 > On Sat, Mar 28, 2020 at 4:12 AM Jan Kiszka mailto:jan.kis...@web.de>
 > >> wrote:
 >
 >     On 28.03.20 08:47, PRAKHAR BANSAL wrote:
 >      > Hi Jan,
 >      >
 >      > Thanks for the reply!
 >      >
 >      > I was only considering the command-line tool "code" for
reference
 >     to the
 >      > Jailhouse kernel API(ioctl calls) because I didn't find a
 >     documentation
 >      > of the Jailhouse kernel APIs.
 >
 >     Right, the IOCTL API is not documented so far. It is
currently only used
 >     inside the Jailhouse project. This needs to be formalized
when there
 >     shall be external users like a libvirt driver.
 >
 >     That might be a nice small contribution task: Create some
 >     Documentation/driver-interfaces.md that describes the IOCTLs
along with
 >     their parameter structures and that also includes the current
 >     sysfs-entries.txt as a section. Then send this as patch here.
I'll help
 >     out when details are not clear from reading the code.
 >
 > Sure. I will do that.
 >
 >      >
 >      > For the second part as you mentioned that Jailhouse can
only create
 >      > cells with the constraints defined in the root cell
configuration. I
 >      > have a few questions regarding that.
 >      >
 >      > 1. Is there a way to know if Jailhouse is enabled on the
host and get
 >      > the root cell configuration(s) from Jailhouse through an API?
 >     This can
 >      > be used while binding the libvirt to the Jailhouse hypervisor.
 >
 >     Look at
 >

https://github.com/siemens/jailhouse/blob/master/Documentation/sysfs-entries.txt
 >     for what is reported as runtime information. Full
configurations can't
 >     be read back at this point. This might be reconsidered in the
light of
 >     [1], but I wouldn't plat for that yet.
 >
 >
 > Ok, sure. I am looking into it.
 >
 >
 >      >
 >      > 2. If Jailhouse is not enabled(again can we know this
using some API)
 >      > then, can libvirt enable/disable Jailhouse during the libvirt
 >     binding of
 >      > the Jailhouse driver with a given set of Jailhouse cell
 >     configurations
 >      > describing a complete partitioned system?
 >
 >     With the API above and a given configuration set, yes. The
config set
 >     would have to be provided to the libvirt driver in some
to-be-defined
 >     way (e.g. /etc/libvirt/jailhouse.conf ->
/etc/libvirt/jailhouse/*.cell).
 >
 > Cool, got it. Thanks!
 >
 >      >
 >      > 3. I was wondering, as you mentioned that libvirt driver
should check
 >      > for mismatch of the cell configuration with the root cell
 >     configuration,
 >      > the question is, isn't that done by Jailhouse itself? If
yes, then
 >      > libvirt can just pass on the cell creation requests to
Jailhouse and
 >      > return the response to the user as it is, rather than driver
 >     doing any
 >      > such mismatch check.
 >
 >     With matching I'm referring to a libvirt user request like
"create a
 >     domain with 2 CPUs", while there are no cells left that have
more than
 >     one CPU. Or "give the domain 1G RAM", and you need to find an
available
 >     cell with that much memory. Those are simple examples. A
request that
 >     states "connect the domain to the host network A" implies
that a cell
 >     has a shared-memory link to, say, the root cell that can be
configured
 >     to bridge this. But let's keep that for later and start as
simple as
 >     possible.
 >
 >
 >