ANNOUNCE: forthcoming retirement of virt-tools mailing list
Hello Virt Tools Community For the past 14-ish years this mailing list has served as a place for accepting patches and having discussions for various virtualization userspace projects that didn't justify their own dedicated mailing list. Mostly this was virt-manager and virt-viewer, but also some other smaller tools. Unfortunately as a consequence of infrastructure changes, the redhat.com Mailman service is scheduled to be decomissioned, and in the very near future ($low weeks) will cease to be able to send / receive mail for this list. In recent years development for the virt-viewer and virt-manager projects has largely shifted over to the git forges, via a combination of merge requests for patch submission, and issues for bug/feature discussions. The remaining traffic on this list isn't high enough to justify the setup of new hosting for its continued usage. With this in mind, when the redhat.com Mailman service ends, this mailing list will be retired. For the sake of historical record, the archives will be preserved. As a reminder the primary project websites are: - https://libvirt.org - https://libguestfs.org - https://virt-manager.org - https://libosinfo.org and their assocaited respositories - https://gitlab.com/libvirt - https://gitlab.com/libosinfo - https://github.com/virt-manager - https://gitlab.com/virt-viewer - https://github.com/libguestfs - https://gitlab.com/nbdkit Note, not all of the above use a merge/pull request workflow. Check the individual websites / repositories for guidance. With 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: Does Xen dom0 count as a VM?
On Wed, Jul 19, 2023 at 10:12:07AM +, Sergio Gelato wrote: > Does this project share the view expressed in > https://github.com/systemd/systemd/issues/28113#issuecomment-1621986461 > that "when it comes to systemd's PoV xen dom0 is not a VM. i.e. the VM > that owns the hardware is not a VM in our eyes"? That is correct. Domain 0 in a Xen-like hypervisor is special, it is the equivalent of the Lnux host OS in KVM world and clearly should not be considered a VM. > If so, virt-what has a similar bug to the above-mentioned systemd issue. There is a code comment in one probing scenario that it cannot distinguish domu/dom0. With 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 :|
[PATCH 1/5] virt-what-cvm: check if hypervisor bit is set
Before doing any probes for a confidential VM, check that the
tool is running under a hypervisor, rather than bare metal
Signed-off-by: Daniel P. Berrangé
---
virt-what-cvm.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/virt-what-cvm.c b/virt-what-cvm.c
index 407efb4..f184768 100644
--- a/virt-what-cvm.c
+++ b/virt-what-cvm.c
@@ -35,6 +35,9 @@ static bool dodebug = false;
#define debug(...) do { if (dodebug) fprintf(stderr, __VA_ARGS__); } while(0)
+
+#define CPUID_PROCESSOR_INFO_AND_FEATURE_BITS 0x1
+
/*
* AMD64 Architecture Programmer’s Manual Volume 3:
* General-Purpose and System Instructions.
@@ -72,6 +75,9 @@ static bool dodebug = false;
#define CPUID_SIG_INTEL "GenuineIntel"
#define CPUID_SIG_INTEL_TDX "IntelTDX"
+/* ecx bit 31: set => hyperpvisor, unset => bare metal */
+#define CPUID_FEATURE_HYPERVISOR (1 << 31)
+
/*
* This TPM NV data format is not explicitly documented anywhere,
* but the header definition is present in code at:
@@ -335,11 +341,32 @@ cpu_sig_intel (void)
puts ("intel-tdx");
}
+static bool
+cpu_is_hv (void)
+{
+ uint32_t eax, ebx, ecx, edx;
+ bool is_hv;
+
+ eax = CPUID_PROCESSOR_INFO_AND_FEATURE_BITS;
+ ebx = ecx = edx = 0;
+
+ cpuid(, , , );
+
+ is_hv = ecx & CPUID_FEATURE_HYPERVISOR;
+
+ debug ("CPUID is hypervisor: %s\n", is_hv ? "yes" : "no");
+ return is_hv;
+}
+
static void
cpu_sig (void)
{
char sig[13];
+ /* Skip everything on bare metal */
+ if (!cpu_is_hv ())
+return;
+
memset (sig, 0, sizeof sig);
cpuid_leaf (0, sig);
--
2.40.1
[PATCH 0/5] virt-what-cvm: improve SEV-SNP detection on Azure/HyperV
My first attempt used a dirty hack of looking at the TPM NV index for existance of a SEV-SNP attestation report. I've since learnt of some HyperV specific CPUID leafs that can be used to detect SEV-SNP in a much simlper/saner manner. Thus this series drops the TPM code and replaces it with CPUID. The fact name is also changed to 'hyperv-' instead of 'azure-' since it is really a property of the hypervisor rather than cloud service. Daniel P. Berrangé (5): virt-what-cvm: check if hypervisor bit is set virt-what-cvm: support alternative cpuid leaf ordering virt-what-cvm: probe for SNP/HCL on HyperV/Azure via CPUID virt-what-cvm: drop TPM logic for detecting SNP on HyperV/Azure virt-what-cvm: rename 'azure-hcl' fact to 'hyperv-hcl' Makefile.am | 3 - configure.ac | 2 - virt-what-cvm.c | 231 +- virt-what-cvm.pod | 6 +- 4 files changed, 88 insertions(+), 154 deletions(-) -- 2.40.1
[PATCH 4/5] virt-what-cvm: drop TPM logic for detecting SNP on HyperV/Azure
Now we have proper CPUID detection, we no longer need the TPM
hacks.
Signed-off-by: Daniel P. Berrangé
---
Makefile.am | 3 -
configure.ac| 2 -
virt-what-cvm.c | 161 ++--
3 files changed, 6 insertions(+), 160 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 2050bef..b68540f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -30,9 +30,6 @@ if HOST_CPU_IA64
libexec_PROGRAMS += virt-what-ia64-xen-rdtsc-test
endif
-virt_what_cvm_LDADD = $(TPM2_TSS_LIBS)
-virt_what_cvm_CFLAGS = $(TPM2_TSS_CFLAGS)
-
if HAVE_POD2MAN
CLEANFILES += virt-what.1 virt-what-cvm.1 virt-what.txt virt-what-cvm.txt
diff --git a/configure.ac b/configure.ac
index 4418483..ab8eb75 100644
--- a/configure.ac
+++ b/configure.ac
@@ -32,8 +32,6 @@ dnl Architecture we are compiling for.
AC_CANONICAL_HOST
AM_CONDITIONAL([HOST_CPU_IA64], [ test "x$host_cpu" = "xia64" ])
-PKG_HAVE_DEFINE_WITH_MODULES(TPM2_TSS, tss2-esys, [tpm2-tss package])
-
dnl List of tests.
tests="\
diff --git a/virt-what-cvm.c b/virt-what-cvm.c
index a7a224f..8b8a4df 100644
--- a/virt-what-cvm.c
+++ b/virt-what-cvm.c
@@ -26,10 +26,6 @@
#include
#include
#include
-#ifdef HAVE_TPM2_TSS
-#include
-#include
-#endif
static bool dodebug = false;
@@ -97,121 +93,8 @@ static bool dodebug = false;
#define CPUID_HYPERV_ISOLATION_TYPE_MASK 0xf
#define CPUID_HYPERV_ISOLATION_TYPE_SNP 2
-/*
- * This TPM NV data format is not explicitly documented anywhere,
- * but the header definition is present in code at:
- *
- *
https://github.com/kinvolk/azure-cvm-tooling/blob/main/az-snp-vtpm/src/hcl.rs
- */
-#define TPM_AZURE_HCLA_REPORT_INDEX 0x0141
-
-struct TPMAzureHCLAHeader {
- uint32_t signature;
- uint32_t version;
- uint32_t report_len;
- uint32_t report_type;
- uint32_t unknown[4];
-};
-
-/* The bytes for "HCLA" */
-#define TPM_AZURE_HCLA_SIGNATURE 0x414C4348
-#define TPM_AZURE_HCLA_VERSION 0x1
-#define TPM_AZURE_HCLA_REPORT_TYPE_SNP 0x2
-
#if defined(__x86_64__)
-#ifdef HAVE_TPM2_TSS
-static char *
-tpm_nvread(uint32_t nvindex, size_t *retlen)
-{
- TSS2_RC rc;
- ESYS_CONTEXT *ctx = NULL;
- ESYS_TR primary = ESYS_TR_NONE;
- ESYS_TR session = ESYS_TR_NONE;
- ESYS_TR nvobj = ESYS_TR_NONE;
- TPM2B_NV_PUBLIC *pubData = NULL;
- TPMT_SYM_DEF sym = {
-.algorithm = TPM2_ALG_AES,
-.keyBits = { .aes = 128 },
-.mode = { .aes = TPM2_ALG_CFB }
- };
- char *ret;
- size_t retwant;
-
- rc = Esys_Initialize(, NULL, NULL);
- if (rc != TSS2_RC_SUCCESS)
-return NULL;
-
- rc = Esys_Startup(ctx, TPM2_SU_CLEAR);
- debug("tpm startup %d\n", rc);
- if (rc != TSS2_RC_SUCCESS)
-goto error;
-
- rc = Esys_StartAuthSession(ctx, ESYS_TR_NONE, ESYS_TR_NONE,
-ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE,
-NULL, 0,
-, TPM2_ALG_SHA256, );
- debug("tpm auth session %d\n", rc);
- if (rc != TSS2_RC_SUCCESS)
-goto error;
-
- rc = Esys_TR_FromTPMPublic(ctx, nvindex, ESYS_TR_NONE,
-ESYS_TR_NONE, ESYS_TR_NONE, );
- debug("tpm from public %d\n", rc);
- if (rc != TSS2_RC_SUCCESS)
-goto error;
-
- rc = Esys_NV_ReadPublic(ctx, nvobj, ESYS_TR_NONE,
- ESYS_TR_NONE, ESYS_TR_NONE,
- , NULL);
- debug("tpm read public %d\n", rc);
- if (rc != TPM2_RC_SUCCESS)
-goto error;
-
- retwant = pubData->nvPublic.dataSize;
- free(pubData);
- *retlen = 0;
- ret = malloc(retwant);
- assert(ret);
- while (*retlen < retwant) {
-size_t want = retwant - *retlen;
-TPM2B_MAX_NV_BUFFER *data = NULL;
-if (want > 1024)
- want = 1024;
-rc = Esys_NV_Read(ctx, ESYS_TR_RH_OWNER, nvobj, session, ESYS_TR_NONE,
ESYS_TR_NONE,
- want, *retlen, );
-debug("tpm nv read %d\n", rc);
-if (rc != TPM2_RC_SUCCESS) {
- free(ret);
- goto error;
-}
-
-memcpy(ret + *retlen, data->buffer, data->size);
-*retlen += data->size;
-free(data);
- }
-
- return ret;
-
- error:
- if (nvobj != ESYS_TR_NONE)
-Esys_FlushContext(ctx, nvobj);
- if (session != ESYS_TR_NONE)
-Esys_FlushContext(ctx, session);
- if (primary != ESYS_TR_NONE)
-Esys_FlushContext(ctx, primary);
- Esys_Finalize();
- *retlen = 0;
- return NULL;
-}
-#else /* ! HAVE_TPM2_TSS */
-static char *
-tpm_nvread(uint32_t nvindex, size_t *retlen)
-{
- return NULL;
-}
-#endif /* ! HAVE_TPM2_TSS */
-
/* Copied from the Linux kernel definition in
* arch/x86/include/asm/processor.h
*/
@@ -263,34 +146,6 @@ msr (off_t index)
return ret;
}
-bool
-cpu_sig_amd_azure (void)
-{
- size_t datalen = 0;
- char *data = tpm_nvread(TPM_AZURE_HCLA_REPORT_INDEX, );
- struct TPMAzureHCLAHeader *header = (struct TPMAzureHCLAHeader *)data;
- bool ret;
-
- if (!data)
-return false;
-
- if (datalen < sizeof(struct TPM
[PATCH 5/5] virt-what-cvm: rename 'azure-hcl' fact to 'hyperv-hcl'
Azure is a cloud service that uses the HyperV platform, so we
should refer to the fact as 'hyperv-hcl', not 'azure-hcl'.
Signed-off-by: Daniel P. Berrangé
---
virt-what-cvm.c | 2 +-
virt-what-cvm.pod | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/virt-what-cvm.c b/virt-what-cvm.c
index 8b8a4df..52b3426 100644
--- a/virt-what-cvm.c
+++ b/virt-what-cvm.c
@@ -214,7 +214,7 @@ cpu_sig_amd (void)
if (cpu_sig_amd_hyperv ()) {
puts ("amd-sev-snp");
- puts ("azure-hcl");
+ puts ("hyperv-hcl");
} else {
debug("No hyperv CPUID\n");
}
diff --git a/virt-what-cvm.pod b/virt-what-cvm.pod
index 12cfc6a..0f90765 100644
--- a/virt-what-cvm.pod
+++ b/virt-what-cvm.pod
@@ -52,11 +52,11 @@ This is a confidential guest running with Intel TDX
technology
Status: tested on Microsoft Azure TDX CVM (preview)
-=item B
+=item B
This is a confidential guest running unenlightened under the
-Azure HCL (Host Compatibility Layer). This will be paired with
-B.
+HyperV (Azure) HCL (Host Compatibility Layer). This will be
+paired with B.
Status: tested on Microsoft Azure SEV-SNP CVM
--
2.40.1
[PATCH 2/5] virt-what-cvm: support alternative cpuid leaf ordering
The HyperV CPUID leaf for reporting the vendor string has an
alternative ordering of ecx/edx.
Signed-off-by: Daniel P. Berrangé
---
virt-what-cvm.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/virt-what-cvm.c b/virt-what-cvm.c
index f184768..1e7c50b 100644
--- a/virt-what-cvm.c
+++ b/virt-what-cvm.c
@@ -209,11 +209,14 @@ cpuid (uint32_t *eax, uint32_t *ebx, uint32_t *ecx,
uint32_t *edx)
static uint32_t
-cpuid_leaf (uint32_t eax, char *sig)
+cpuid_leaf (uint32_t eax, char *sig, bool swapped)
{
uint32_t *sig32 = (uint32_t *) sig;
- cpuid (, [0], [2], [1]);
+ if (swapped)
+cpuid (, [0], [2], [1]);
+ else
+cpuid (, [0], [1], [2]);
sig[12] = 0; /* \0-terminate the string to make string comparison possible */
debug("CPUID sig %s\n", sig);
return eax;
@@ -335,7 +338,7 @@ cpu_sig_intel (void)
return;
memset (sig, 0, sizeof sig);
- cpuid_leaf (CPUID_INTEL_TDX_ENUMERATION, sig);
+ cpuid_leaf (CPUID_INTEL_TDX_ENUMERATION, sig, true);
if (memcmp (sig, CPUID_SIG_INTEL_TDX, sizeof(sig)) == 0)
puts ("intel-tdx");
@@ -368,7 +371,7 @@ cpu_sig (void)
return;
memset (sig, 0, sizeof sig);
- cpuid_leaf (0, sig);
+ cpuid_leaf (0, sig, true);
if (memcmp (sig, CPUID_SIG_AMD, sizeof(sig)) == 0)
cpu_sig_amd ();
--
2.40.1
[PATCH 3/5] virt-what-cvm: probe for SNP/HCL on HyperV/Azure via CPUID
When running a confidential VM on Azure (HyperV) we can probe
CPUID leaf 0x4003 to detect if VM isolation is present,
and 0x400c to detect what kind of isolation is used.
Signed-off-by: Daniel P. Berrangé
---
virt-what-cvm.c | 62 +++--
1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/virt-what-cvm.c b/virt-what-cvm.c
index 1e7c50b..a7a224f 100644
--- a/virt-what-cvm.c
+++ b/virt-what-cvm.c
@@ -70,14 +70,33 @@ static bool dodebug = false;
#define CPUID_INTEL_TDX_ENUMERATION 0x21
+/* Requirements for Implementing the Microsoft Hypervisor Interface
+ *
https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/tlfs
+ */
+#define CPUID_HYPERV_VENDOR_AND_MAX_FUNCTIONS 0x4000
+
+#define CPUID_HYPERV_FEATURES 0x4003
+
+#define CPUID_HYPERV_ISOLATION_CONFIG 0x400C
+
+#define CPUID_HYPERV_MIN 0x4005
+#define CPUID_HYPERV_MAX 0x4000
#define CPUID_SIG_AMD "AuthenticAMD"
#define CPUID_SIG_INTEL "GenuineIntel"
#define CPUID_SIG_INTEL_TDX "IntelTDX"
+#define CPUID_SIG_HYPERV"Microsoft Hv"
/* ecx bit 31: set => hyperpvisor, unset => bare metal */
#define CPUID_FEATURE_HYPERVISOR (1 << 31)
+/* Linux include/asm-generic/hyperv-tlfs.h */
+#define CPUID_HYPERV_CPU_MANAGEMENT (1 << 12) /* root partition */
+#define CPUID_HYPERV_ISOLATION (1 << 22) /* confidential VM partition */
+
+#define CPUID_HYPERV_ISOLATION_TYPE_MASK 0xf
+#define CPUID_HYPERV_ISOLATION_TYPE_SNP 2
+
/*
* This TPM NV data format is not explicitly documented anywhere,
* but the header definition is present in code at:
@@ -272,6 +291,44 @@ cpu_sig_amd_azure (void)
return ret;
}
+static bool
+cpu_sig_amd_hyperv (void)
+{
+ uint32_t eax, ebx, ecx, edx;
+ char sig[13];
+ uint32_t feat;
+
+ feat = cpuid_leaf (CPUID_HYPERV_VENDOR_AND_MAX_FUNCTIONS, sig, false);
+
+ if (feat < CPUID_HYPERV_MIN ||
+ feat > CPUID_HYPERV_MAX)
+return false;
+
+ if (memcmp (sig, CPUID_SIG_HYPERV, sizeof(sig)) != 0)
+return false;
+
+ debug ("CPUID is on hyperv\n");
+ eax = CPUID_HYPERV_FEATURES;
+ ebx = ecx = edx = 0;
+
+ cpuid(, , , );
+
+ if (ebx & CPUID_HYPERV_ISOLATION &&
+ !(ebx & CPUID_HYPERV_CPU_MANAGEMENT)) {
+
+eax = CPUID_HYPERV_ISOLATION_CONFIG;
+ebx = ecx = edx = 0;
+cpuid(, , , );
+
+if ((ebx & CPUID_HYPERV_ISOLATION_TYPE_MASK) ==
+ CPUID_HYPERV_ISOLATION_TYPE_SNP) {
+ return true;
+}
+ }
+
+ return false;
+}
+
static void
cpu_sig_amd (void)
{
@@ -298,9 +355,10 @@ cpu_sig_amd (void)
* exposes a SEV-SNP attestation report as evidence.
*/
if (!(eax & (1 << 1))) {
-debug ("No sev in CPUID, try azure TPM NV\n");
+debug ("No sev in CPUID, try hyperv CPUID/azure TPM NV\n");
-if (cpu_sig_amd_azure()) {
+if (cpu_sig_amd_hyperv () ||
+cpu_sig_amd_azure()) {
puts ("amd-sev-snp");
puts ("azure-hcl");
} else {
--
2.40.1
[PATCH virt-what] Introduce 'virt-what-cvm' program
The 'virt-what' program prints facts that reflect the hypervisor that
the guest is running under.
The new complementary 'virt-what-cvm' program prints facts that reflect
the confidential virtualization technology the guest is running under,
if any.
It is kept as a separate tool, rather than incorporating the facts into
'virt-what' output because it is considering a different aspect of the
virtualization. Furthermore there are specific security concerns around
the usage of facts reported by 'virt-what-cvm'.
The tool has been tested in a number of environments
* Azure confidential guest with AMD SEV-SNP (GA)
* Azure confidential guest with Intel TDX (technology preview)
* Fedora 37 QEMU/KVM guest with AMD SEV (GA)
* Fedora 37 QEMU/KVM guest with AMD SEV-ES (GA)
* Fedora 38 QEMU/KVM guest with AMD SEV-SNP + SVSM (devel snapshot)
Signed-off-by: Daniel P. Berrangé
---
.gitignore| 3 +
Makefile.am | 12 +-
configure.ac | 3 +
virt-what-cvm.c | 404 ++
virt-what-cvm.pod | 195 ++
5 files changed, 613 insertions(+), 4 deletions(-)
create mode 100644 virt-what-cvm.c
create mode 100644 virt-what-cvm.pod
diff --git a/.gitignore b/.gitignore
index 4833fd6..ba897a1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -26,5 +26,8 @@ Makefile.in
/test-driver
/virt-what
/virt-what-cpuid-helper
+/virt-what-cvm
+/virt-what-cvm.1
+/virt-what-cvm.txt
/virt-what.1
/virt-what.txt
diff --git a/Makefile.am b/Makefile.am
index 5435132..2050bef 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,20 +24,24 @@ EXTRA_DIST = .gitignore virt-what.in virt-what.pod
SUBDIRS = . tests
sbin_SCRIPTS = virt-what
+sbin_PROGRAMS = virt-what-cvm
libexec_PROGRAMS = virt-what-cpuid-helper
if HOST_CPU_IA64
libexec_PROGRAMS += virt-what-ia64-xen-rdtsc-test
endif
+virt_what_cvm_LDADD = $(TPM2_TSS_LIBS)
+virt_what_cvm_CFLAGS = $(TPM2_TSS_CFLAGS)
+
if HAVE_POD2MAN
-CLEANFILES += virt-what.1 virt-what.txt
-man_MANS = virt-what.1
+CLEANFILES += virt-what.1 virt-what-cvm.1 virt-what.txt virt-what-cvm.txt
+man_MANS = virt-what.1 virt-what-cvm.1
-virt-what.1: virt-what.pod
+%.1: %.pod
pod2man -c "Virtualization Support" --release "$(PACKAGE)-$(VERSION)" \
$? > $@
-virt-what.txt: virt-what.pod
+%.txt: %.pod
pod2text $? > $@
endif
diff --git a/configure.ac b/configure.ac
index a28a716..77b7665 100644
--- a/configure.ac
+++ b/configure.ac
@@ -32,6 +32,9 @@ dnl Architecture we are compiling for.
AC_CANONICAL_HOST
AM_CONDITIONAL([HOST_CPU_IA64], [ test "x$host_cpu" = "xia64" ])
+PKG_HAVE_DEFINE_WITH_MODULES(TPM2_TSS, tss2-esys, [tpm2-tss package])
+
+
dnl List of tests.
tests="\
alibaba-cloud-arm \
diff --git a/virt-what-cvm.c b/virt-what-cvm.c
new file mode 100644
index 000..407efb4
--- /dev/null
+++ b/virt-what-cvm.c
@@ -0,0 +1,404 @@
+/* virt-what-cvm-helper: Are we running inside confidential VM
+ * Copyright (C) 2023 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include "config.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#ifdef HAVE_TPM2_TSS
+#include
+#include
+#endif
+
+static bool dodebug = false;
+
+#define debug(...) do { if (dodebug) fprintf(stderr, __VA_ARGS__); } while(0)
+
+/*
+ * AMD64 Architecture Programmer’s Manual Volume 3:
+ * General-Purpose and System Instructions.
+ * Chapter: E4.1 - Maximum Extended Function Number and Vendor String
+ * https://www.amd.com/system/files/TechDocs/24594.pdf
+ */
+#define CPUID_GET_HIGHEST_FUNCTION 0x8000
+
+/*
+ * AMD64 Architecture Programmer’s Manual Volume 3:
+ * General-Purpose and System Instructions.
+ * Chapter: E4.17 - Encrypted Memory Capabilities
+ * https://www.amd.com/system/files/TechDocs/24594.pdf
+ */
+#define CPUID_AMD_GET_ENCRYPTED_MEMORY_CAPABILITIES 0x801f
+
+/*
+ * AMD64 Architecture Programmer’s Manual Volume 3:
+ * General-Purpose and System Instructions.
+ * Chapter: 15.34.10 - SEV_STATUS MSR
+ * https://www.amd.com/system/files/TechDocs/24593.pdf
+ */
+#define MSR_AMD64_SEV 0xc0010131
+
+/*
+ * Intel® TDX Module v1.5 Base Architecture Specification
+ * Chapter: 11.2
+ *
https://www.intel.com/content/www/us/en/c
Re: virt-manager upgrade
On Fri, Oct 07, 2022 at 10:21:18AM +0100, Richard W.M. Jones wrote: > On Thu, Oct 06, 2022 at 07:14:39PM +, woodcab wrote: > > Hello, > > > > Still new to Linux and had a few questions regarding upgrading to 4.1.0 > > > > I'm using virt-manager 2.2.1 on Ubuntu MATE 20.4. There is an > > upgrade available for Ubuntu Mate to MATE 20.4.1. I am told that > > this is a minor upgrade and that it can be done with my files in > > place. I have backed up my files and VMs along with the .xml > > files. My question is how to best proceed with that upgrade? If > > virt-manager is upgraded to 4.1.0 in the process, will my VMs that > > were setup in 2.2.1 still work in 4.1.0? Or should I upgrade > > virt-manager to an intermediate version before going to 4.1.0? > > I don't think I've ever had to worry about existing VMs when upgrading > libvirt or virt-manager (BTW I think *libvirt* is more relevant to > this since that's what is actually managing the VMs). I have upgraded > libvirt many times on my system that runs VMs even while the VMs were > running. Libvirt goes to quite alot of trouble to ensure you can upgrade the host software stack and not cause problems to your VMs. You can even upgrade libvirt itself, while QEMU VMs are running, and it should seemlessly reconnect. Upgrading virt-manger itself will not have any impact of VMs, since virt-manager is merely a GUI frontend. Downgrading software, however, is totally unsupported, and you get to keep any pieces & put it back together if it breaks :-) > Since you've done the right thing by backing up the existing VMs and > *.xml files, you should be good even in the unlikely case that > anything goes wrong during the upgrade, so I guess go for it! With 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: Support for IPv6 NAT Mode
On Wed, Aug 24, 2022 at 10:22:29AM +0530, vaishu venkat wrote: > Hello Daniel, > > Thanks for the quick response. > > By changing the above lines in the xml file, when applying/saving the > changes, it reverts back. Still I could notice the FORWARDING mode as > routed, not as NAT for IPv6. Please find the attached screenshot for your > reference. > > Please share your observations. Note changes in the XML only apply once you stop and then start the network again - it won't be visible immediately after saving/applying. With 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: How can I disable secure boot using virt-install cli?
On Wed, Aug 03, 2022 at 11:00:30AM -0500, Andrea Bolognani wrote: > On Wed, Aug 03, 2022 at 01:17:33PM +0800, Lucas Liu wrote: > > Hello all: > > > > I am looking for a way to disable secure boot for UEFI guests: > > In 3.2.0 I use the command blow to achieve it: > > > > # virt-install --name GuestOne --location #URL --machine q35 --vcpus=2 > > --memory 4096 --file-size=20 --boot uefi --boot > > nvram.template=/usr/share/edk2/ovmf/OVMF_VARS.fd > > > > However, in 4.0.0 I cannot get the same result for this cmd > > > > Expect VM is booted with secureboot disabled. But the actual result is the > > VM is booted with secureboot enabled. > > > > # mokutil --sb-state > > SecureBoot enabled > > > > ... > > > > hvm > > > type='pflash'>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd > > > template='/usr/share/edk2/ovmf/OVMF_VARS.secboot.fd'>/var/lib/libvirt/qemu/nvram/rhel9_VARS.fd > > > > > > ... > > > > It seems it still creates guests with > > "/usr/share/edk2/ovmf/OVMF_VARS.secboot.fd" as the nvram template. > > This should do what you want: > > --boot > uefi,firmware.feature0.name=enrolled-keys,firmware.feature0.enabled=no,firmware.feature1.name=secure-boot,firmware.feature1.enabled=yes > > A bit of a mouthful, I know :) The equivalent XML snippet would be > > > > > > > This seems to kas to leave secureboot enabled, but with no enrolled keys. To disable secureboot fully I use this --boot firmware=efi,firmware.feature0.enabled=no,firmware.feature0.name=secure-boot \ With 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: [PATCH] detect AWS arm virtual instance as kvm
On Wed, May 25, 2022 at 02:34:53PM +0800, Frank Liang wrote:
> Hello,
>
> aws arm virtual instances are running on top of Nitro hypervisor(KVM-based
> hypervisor).
> I am proposing this patch to keep virt-what output consistent with x86
> virtual instances.
> Please review it. Thanks
>
> With this patch:
> $ sudo virt-what
> kvm
> aws
> $ cat /sys/devices/virtual/dmi/id/product_name
> t4g.small
>
> Without it:
> $ sudo virt-what
> aws
>
> Here is the test pass log with this patch.
> # cat test-suite.log
> ==
>virt-what 1.22: tests/aws-kvm-arm/test-suite.log
> ==
>
> # TOTAL: 1
> # PASS: 1
> # SKIP: 0
> # XFAIL: 0
> # FAIL: 0
> # XPASS: 0
> # ERROR: 0
>
> .. contents:: :depth: 2
>
> Rgs,
> Frank
> From 931e5f24b9e7e6e61cebe3213166bd691df80db0 Mon Sep 17 00:00:00 2001
> From: Xiao Liang
> Date: Tue, 24 May 2022 17:34:52 +0800
> Subject: [PATCH] detect AWS arm virtual instance as kvm
>
> AWS arm virtual instance is KVM based hypervisor.
> With this patch:
> $ sudo virt-what
> kvm
> aws
> $ cat /sys/devices/virtual/dmi/id/product_name
> t4g.small
>
> Without it:
> $ sudo virt-what
> aws
>
> Signed-off-by: Xiao Liang
> ---
> tests/aws-kvm-arm/test.sh | 3 ++-
> virt-what.in | 5 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tests/aws-kvm-arm/test.sh b/tests/aws-kvm-arm/test.sh
> index 5d31995..d469a99 100755
> --- a/tests/aws-kvm-arm/test.sh
> +++ b/tests/aws-kvm-arm/test.sh
> @@ -18,7 +18,8 @@
> output="$(PATH=../..:$PATH virt-what --test-root=. 2>&1)"
> # XXX At the moment KVM cannot be detected on this platform.
> # We hope to fix that, but for now the only fact printed is "aws".
This comment looks outdated given the change you made
> -expected="aws"
> +expected="kvm
> +aws"
>
> if [ "$output" != "$expected" ]; then
> echo "$0: test failed because output did not match expected"
> diff --git a/virt-what.in b/virt-what.in
> index b59714e..9afbd40 100644
> --- a/virt-what.in
> +++ b/virt-what.in
> @@ -339,6 +339,11 @@ if ! "$skip_qemu_kvm"; then
> echo qemu
> skip_lkvm=true
> fi
> +if echo "$dmi" | grep -q 'Amazon EC2' &&
> +echo "$dmi" | grep -q 'System is a virtual machine'; then
> +echo kvm
> +skip_lkvm=true
> +fi
> elif [ -d ${root}/proc/device-tree/hypervisor ] &&
> grep -q "linux,kvm" /proc/device-tree/hypervisor/compatible; then
> # We are running as a spapr KVM guest on ppc64
> --
> 2.36.1
>
With 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: no release tarball
On Thu, Apr 21, 2022 at 01:30:29PM +0200, Henrik Riomar wrote:
> On Thu, Apr 21, 2022 at 1:11 PM Richard W.M. Jones wrote:
> >
> > On Thu, Apr 21, 2022 at 12:54:42PM +0200, Henrik Riomar wrote:
>
> > > I now note that commit b7cc3d93a613ef6d0ac5ccd6e32cc3d66e057243, that
> > > is part of v1.22 introduced two bashisms:
> > >
> > > possible bashism in ./virt-what line 119 (echo -e):
> > > │
> > > if ( { echo -e "GET /latest/meta-datainstance/instance-type
> > > HTTP/1.0\r\nHost: 100.100.100.200\r\n\r" │
> > > >&3; grep -sq 'ebm' <&3 ; } 3<> /dev/tcp/100.100.100.200/80 ) 2>/dev/null
> > > >; then │
> > > possible bashism in ./virt-what line 119 (/dev/(tcp|udp)):
> > > │
> > > if ( { echo -e "GET /latest/meta-datainstance/instance-type
> > > HTTP/1.0\r\nHost: 100.100.100.200\r\n\r" │
> > > >&3; grep -sq 'ebm' <&3 ; } 3<> /dev/tcp/100.100.100.200/80 ) 2>/dev/null
> > > >; then
> > >
> > > Can this be fixed? as that is the only bashisms in virt-what.
> >
> > Can you send a patch to fix it?
>
> The echo -e should be a simple replace with printf "%b\n", but the
> code using file descriptor 3 and the bash feature to open a tcp
> socket, needs some more work,
> it can be replaced with nc, but the problem is that I have no Alibaba
> Cloud VM to test a fix on.
Or just change the first line of the script to #!/bin/bash, so it
doesn't rely on the default shell impl.
With 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: Add support for enabling Secure Encrypted Virtualization in the GUI
On Wed, Apr 20, 2022 at 09:11:27AM -0400, Cole Robinson wrote: > On 4/19/22 1:21 PM, Daniel P. Berrangé wrote: > > On Tue, Apr 19, 2022 at 12:05:29PM -0400, Cole Robinson wrote: > >> On 4/4/22 11:49 AM, Charles Arnold wrote: > >>> > >>> > >>> On 4/4/22 6:50 AM, Daniel P. Berrangé wrote: > >>>> On Fri, Apr 01, 2022 at 12:13:17PM -0600, Charles Arnold wrote: > >>>>> From d700e8cee7cd525c0022b5a9a440f64c4ab149f0 Mon Sep 17 00:00:00 2001 > >>>>> From: Charles Arnold > >>>>> Date: Fri, 1 Apr 2022 12:01:21 -0600 > >>>>> Subject: [PATCH 1/1] Add support for enabling Secure Encrypted > >>>>> Virtualization > >>>>> in the GUI > >>>>> > >>>>> Add an "Enable Launch Security" checkbox on the Details memory tab. > >>>>> Do the minimal configuration required for libvirt to enable this feature > >>>>> on compatible hardware. > >>>>> > >>>> Don't we need to turn on the 'iommu' option for all virtio devices > >>>> too, and disable PXE on any NICs ? > >>>> > >>>> https://libvirt.org/kbase/launch_security_sev.html#virtio > >>>> > >>> > >>> I used to enumerate through the virtio devices in an old version of this > >>> patch > >>> for virt-manager and enable iommu but it really wasn't reasonable for > >>> virt-manager to track which virtio devices needed iommu enabled. > >>> Additionally, > >>> libvirt will sometimes add a device when a VM is created. This patch > >>> leans on libvirt to do the right thing when sev is enabled similar to what > >>> happens when launch security is specified on the virt-install command > >>> line. > >>> > >> > >> Yeah, I would still like to see libvirt do this unless there's a good > >> reason why it can't. From my July 2020 mail > >> https://listman.redhat.com/archives/virt-tools-list/2020-July/017087.html > >> > >>> if sev > >>> launchSecurity _requires_ every virtio device to have iommu='on' then > >>> libvirt should be setting that itself. It doesn't need to hardcode it > >>> into the XML, it can set it at VM startup time. If the user set an > >>> explicit value in the XML then honor that but otherwise fill it in at > >>> runtime when it is required. Trying to deal with this in an app where we > >>> want to advertise turning the config off is basically an impossible > >>> problem to know if we are going to undo any explicit user config or not. > >> > >> danpb does this sound reasonable? If so I can work on this. > > > > Something automagic sounds ok > > > >> Also, anyone know if TDX and SNP will require virtio iommu setting as well? > > > > I expect SNP will, but no idea about TDX. > > > > So apparently qemu 6.0.0+ already does this for us: > https://gitlab.com/qemu-project/qemu/-/commit/9f88a7a3df > > Which is strange because some of my sev testing definitely failed with > an iommu sounding error in the guest kernel until I enabled iommu on > virtio-blk, but I can't reproduce now. Maybe I was using an older qemu, > I think the host was fedora 34 at the time Or you had new QEMU, but the guest was fixed on the older machine type perhaps With 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: Add support for enabling Secure Encrypted Virtualization in the GUI
On Tue, Apr 19, 2022 at 12:05:29PM -0400, Cole Robinson wrote: > On 4/4/22 11:49 AM, Charles Arnold wrote: > > > > > > On 4/4/22 6:50 AM, Daniel P. Berrangé wrote: > >> On Fri, Apr 01, 2022 at 12:13:17PM -0600, Charles Arnold wrote: > >>> From d700e8cee7cd525c0022b5a9a440f64c4ab149f0 Mon Sep 17 00:00:00 2001 > >>> From: Charles Arnold > >>> Date: Fri, 1 Apr 2022 12:01:21 -0600 > >>> Subject: [PATCH 1/1] Add support for enabling Secure Encrypted > >>> Virtualization > >>> in the GUI > >>> > >>> Add an "Enable Launch Security" checkbox on the Details memory tab. > >>> Do the minimal configuration required for libvirt to enable this feature > >>> on compatible hardware. > >>> > >> Don't we need to turn on the 'iommu' option for all virtio devices > >> too, and disable PXE on any NICs ? > >> > >> https://libvirt.org/kbase/launch_security_sev.html#virtio > >> > > > > I used to enumerate through the virtio devices in an old version of this > > patch > > for virt-manager and enable iommu but it really wasn't reasonable for > > virt-manager to track which virtio devices needed iommu enabled. > > Additionally, > > libvirt will sometimes add a device when a VM is created. This patch > > leans on libvirt to do the right thing when sev is enabled similar to what > > happens when launch security is specified on the virt-install command line. > > > > Yeah, I would still like to see libvirt do this unless there's a good > reason why it can't. From my July 2020 mail > https://listman.redhat.com/archives/virt-tools-list/2020-July/017087.html > > > if sev > > launchSecurity _requires_ every virtio device to have iommu='on' then > > libvirt should be setting that itself. It doesn't need to hardcode it > > into the XML, it can set it at VM startup time. If the user set an > > explicit value in the XML then honor that but otherwise fill it in at > > runtime when it is required. Trying to deal with this in an app where we > > want to advertise turning the config off is basically an impossible > > problem to know if we are going to undo any explicit user config or not. > > danpb does this sound reasonable? If so I can work on this. Something automagic sounds ok > Also, anyone know if TDX and SNP will require virtio iommu setting as well? I expect SNP will, but no idea about TDX. With 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: Add support for enabling Secure Encrypted Virtualization in the GUI
On Mon, Apr 04, 2022 at 06:21:42PM +0200, Boris Fiuczynski wrote: > On 4/4/22 5:48 PM, Charles Arnold wrote: > > On 4/4/22 8:37 AM, Boris Fiuczynski wrote: > > > On 4/4/22 2:50 PM, Daniel P. Berrangé wrote: > > > > On Fri, Apr 01, 2022 at 12:13:17PM -0600, Charles Arnold wrote: > > > > > From d700e8cee7cd525c0022b5a9a440f64c4ab149f0 Mon Sep 17 00:00:00 > > > > > 2001 > > > > > From: Charles Arnold > > > > > Date: Fri, 1 Apr 2022 12:01:21 -0600 > > > > > Subject: [PATCH 1/1] Add support for enabling Secure Encrypted > > > > > Virtualization > > > > > in the GUI > > > > > > > > > > Add an "Enable Launch Security" checkbox on the Details memory tab. > > > > > Do the minimal configuration required for libvirt to enable > > > > > this feature > > > > > on compatible hardware. > > > > > > > > > > > > > Don't we need to turn on the 'iommu' option for all virtio devices > > > > too, and disable PXE on any NICs ? > > > > > > > > https://libvirt.org/kbase/launch_security_sev.html#virtio > > > > > > > > With regards, > > > > Daniel > > > > > > > > > > Hi Arnold, > > > your patch does not take into account that libvirt uses launch > > > security for more types besides sev. > > > > > > > > Good point. I haven't taken into account the s390 case which I can correct. > > I'm not aware of other launch security types besides those two. > > > > - Charles > > > > There has been a patch series for TDX on the mailing list in July 2021 but I > am not sure what finally happened to it. > https://listman.redhat.com/archives/libvir-list/2021-July/221098.html TDX support isn't merged in the Linux kernel/KVM, nor in QEMU, nor OVMF AFAIK, so anything related to libvirt & above is on hold until the lower TDX bits are ready. With 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: Add support for enabling Secure Encrypted Virtualization in the GUI
On Fri, Apr 01, 2022 at 12:13:17PM -0600, Charles Arnold wrote: > From d700e8cee7cd525c0022b5a9a440f64c4ab149f0 Mon Sep 17 00:00:00 2001 > From: Charles Arnold > Date: Fri, 1 Apr 2022 12:01:21 -0600 > Subject: [PATCH 1/1] Add support for enabling Secure Encrypted > Virtualization > in the GUI > > Add an "Enable Launch Security" checkbox on the Details memory tab. > Do the minimal configuration required for libvirt to enable this feature > on compatible hardware. > Don't we need to turn on the 'iommu' option for all virtio devices too, and disable PXE on any NICs ? https://libvirt.org/kbase/launch_security_sev.html#virtio With 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: virt-viewer light theme?
On Fri, Nov 26, 2021 at 02:37:45PM -0800, ToddAndMargo wrote: > Hi All, > > FC35 > Xfce 4.16 > virt-viewer-10.0-4.fc35.x86_64 > > Is there a way to switch virt-viewer's decorations > from its current dark theme to a light or system > theme? You can set an env variable if desired GTK_THEME=Adwaita:light 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: [PATCH virt-manager v2 1/2] tests: add tests to support MDEV name change
On Fri, Nov 12, 2021 at 11:51:48AM +0100, Shalini Chellathurai Saroja wrote: > Add tests to ensure that the UUID of mediated devices are parsed > correctly in both newer and older(<7.3.0) versions of libvirt. > > The node device names of mediated devices are changed from > 'MDEV_$UUID'(eg: mdev_b204c698_6731_4f25_b5f4_894614a05ec0) to > 'MDEV_$UUID_$PARENT'(eg: > mdev_b204c698_6731_4f25_b5f4_894614a05ec0_0_0_0014) in libvirt version > 7.8.0. This change impacted the UUID parsing of mediated devices, > which is fixed with commit 0c146b250384cefd2cc0d76b9e808377ebe5. Commit 0c146b250384cefd2cc0d76b9e808377ebe5 introduced test coverage for this changed approach. I'm not seeing what scenarios this covers that aren't already covered ? There are only two codepaths in get_mdev_uuid() and we test both of them. > > Signed-off-by: Shalini Chellathurai Saroja > --- > tests/data/testdriver/testdriver.xml | 28 > tests/test_nodedev.py| 27 +++ > 2 files changed, 55 insertions(+) > > diff --git a/tests/data/testdriver/testdriver.xml > b/tests/data/testdriver/testdriver.xml > index e4880936..13f40136 100644 > --- a/tests/data/testdriver/testdriver.xml > +++ b/tests/data/testdriver/testdriver.xml > @@ -3679,6 +3679,20 @@ ba > > > > + > + mdev_c6c57090_3490_428e_a7fb_8d557f985717_0_0_0023 > + > /sys/devices/css0/0.0.0023/c6c57090-3490-428e-a7fb-8d557f985717 > + css_0_0_0023 > + > +vfio_mdev > + > + > + > +c6c57090-3490-428e-a7fb-8d557f985717 > + > + > + > + > >ap_matrix >/sys/devices/vfio_ap/matrix > @@ -3712,6 +3726,20 @@ ba > > > > + > + mdev_45938d10_4d5d_4e8f_b5ce_27be07b5bab6_matrix > + > /sys/devices/vfio_ap/matrix/45938d10-4d5d-4e8f-b5ce-27be07b5bab6 > + ap_matrix > + > +vfio_mdev > + > + > + > +45938d10-4d5d-4e8f-b5ce-27be07b5bab6 > + > + > + > + > >mdev_4b20d080_1b54_4048_85b3_a6a62d165c01 > > /sys/devices/pci:00/:00:02.0/4b20d080-1b54-4048-85b3-a6a62d165c01 > diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py > index 41435262..8de0facf 100644 > --- a/tests/test_nodedev.py > +++ b/tests/test_nodedev.py > @@ -135,6 +135,7 @@ def testDASDMdev(): > assert dev.parent == "css_0_0_0023" > assert dev.device_type == "mdev" > assert dev.type_id == "vfio_ccw-io" > +assert dev.get_mdev_uuid() == "8e37ee90-2b51-45e3-9b25-bf8283c03110" > > > def testAPQNMdev(): > @@ -145,6 +146,7 @@ def testAPQNMdev(): > assert dev.parent == "ap_matrix" > assert dev.device_type == "mdev" > assert dev.type_id == "vfio_ap-passthrough" > +assert dev.get_mdev_uuid() == "11f92c9d-b0b0-4016-b306-a8071277f8b9" > > > def testPCIMdev(): > @@ -157,7 +159,32 @@ def testPCIMdev(): > assert dev.type_id == "nvidia-11" > assert dev.get_mdev_uuid() == "4b20d080-1b54-4048-85b3-a6a62d165c01" > > + > # libvirt <7.3.0 doesn't support in the mdev node device xml > +@pytest.mark.skipif(libvirt.getVersion() < 7003000, reason="libvirt version > doesn't support new mdev format") > +def testDASDMdevNewFormat(): > +conn = utils.URIs.open_testdriver_cached() > +devname = "mdev_c6c57090_3490_428e_a7fb_8d557f985717_0_0_0023" > +dev = _nodeDevFromName(conn, devname) > +assert dev.name == devname > +assert dev.parent == "css_0_0_0023" > +assert dev.device_type == "mdev" > +assert dev.type_id == "vfio_ccw-io" > +assert dev.get_mdev_uuid() == "c6c57090-3490-428e-a7fb-8d557f985717" > + > + > +@pytest.mark.skipif(libvirt.getVersion() < 7003000, reason="libvirt version > doesn't support new mdev format") > +def testAPQNMdevNewFormat(): > +conn = utils.URIs.open_testdriver_cached() > +devname = "mdev_45938d10_4d5d_4e8f_b5ce_27be07b5bab6_matrix" > +dev = _nodeDevFromName(conn, devname) > +assert dev.name == devname > +assert dev.parent == "ap_matrix" > +assert dev.device_type == "mdev" > +assert dev.type_id == "vfio_ap-passthrough" > +assert dev.get_mdev_uuid() == "45938d10-4d5d-4e8f-b5ce-27be07b5bab6" > + > + > @pytest.mark.skipif(libvirt.getVersion() < 7003000, reason="libvirt version > doesn't support new mdev format") > def testPCIMdevNewFormat(): > conn = utils.URIs.open_testdriver_cached() > -- > 2.30.2 > 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: Triggering a checkpoint from inside the VM
On Thu, Sep 09, 2021 at 08:31:58AM +, Leek, Jim wrote: > I’ll try snapshot create, I didn’t know about that. > > I understand your security concerns, but they don’t apply in this case. The > target environment is air gapped and isolated. I can’t see any reason to fear > an attack on the VM before an attack on the host. > > 2. Well, I was experimenting with getting libvirt out of the system to > simplify things, but I didn’t have much luck yet. Taking libvirt out of the loop is likely to make your life harder in the long term, especially as you'll no longer be insulated from changes in QEMU's external management interfaces. QEMU may look simple to manage directly at first glance, but it is actually quite a complicated to do it well when you look closely. 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: Triggering a checkpoint from inside the VM
On Thu, Sep 09, 2021 at 12:30:53AM +, Leek, Jim wrote:
> Here's the error I sometimes get on checkpoint/restore. It just happened
> again. In this test I had at least a 40 second break between checkpoint and
> restore, so extra sleep doesn't seem to help much.
>
> Error unpausing domain: Timed out during operation: cannot acquire state
> change lock (held by monitor=qemuDispatchDomainMonitorCommand)
>
> Traceback (most recent call last):
> File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in
> cb_wrapper
> callback(asyncjob, *args, **kwargs)
> File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in tmpcb
> callback(*args, **kwargs)
> File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line
> 66, in newfn
> ret = fn(self, *args, **kwargs)
> File "/usr/share/virt-manager/virtManager/object/domain.py", line 1312, in
> resume
> self._backend.resume()
> File "/usr/lib64/python3.6/site-packages/libvirt.py", line 2174, in resume
> if ret == -1: raise libvirtError ('virDomainResume() failed', dom=self)
> libvirt.libvirtError: Timed out during operation: cannot acquire state change
> lock (held by monitor=qemuDispatchDomainMonitorCommand)
This suggests that the previously executed 'qemu-montor-command' has not
finished running yet.
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: [PATCH v2] virtmanager: Fixed the legacy firmware installation error
On Tue, Jan 12, 2021 at 05:38:48PM +0800, Yu-Chen, Cho wrote:
> From: "Cho, Yu-Chen"
>
> When we choose /usr/share/qemu/bios.bin or /usr/share/qemu/bios-256k.bin
> in Customize configuration before install, show this error message:
>
> Unable to complete install: 'operation failed: unable to find any master var
> store for loader: /usr/share/qemu/bios.bin'
>
> Traceback (most recent call last):
> File "/usr/share/virt-manager/virtManager/asyncjob.py", line 65, in
> cb_wrapper
> callback(asyncjob, *args, **kwargs)
> File "/usr/share/virt-manager/virtManager/createvm.py", line 2081, in
> _do_async_install
> installer.start_install(guest, meter=meter)
> File "/usr/share/virt-manager/virtinst/install/installer.py", line 721, in
> start_install
> domain = self._create_guest(
> File "/usr/share/virt-manager/virtinst/install/installer.py", line 669, in
> _create_guest
> domain = self.conn.createXML(install_xml or final_xml, 0)
> File "/usr/lib64/python3.8/site-packages/libvirt.py", line 4347, in
> createXML
> raise libvirtError('virDomainCreateXML() failed')
> libvirt.libvirtError: operation failed: unable to find any master var store
> for loader: /usr/share/qemu/bios.bin
>
> This patch check the loader if it is include bios.bin and bios-256k.bin.
>
> Signed-off-by: Yu-Chen, Cho
> --
> v2:
> Fixed tabs vs spaces issues.
>
> v1:
> Check the loader if it is include bios.bin and bios-256k.bin.
> ---
> virtManager/object/domain.py | 18 +-
> virtinst/guest.py| 13 +
> 2 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/virtManager/object/domain.py b/virtManager/object/domain.py
> index cc2f506d..2fafe2b4 100644
> --- a/virtManager/object/domain.py
> +++ b/virtManager/object/domain.py
> @@ -671,16 +671,16 @@ class vmmDomain(vmmLibvirtObject):
>
> if loader != _SENTINEL:
> if loader is None:
> -# Implies seabios, aka the default, so clear everything
> -guest.os.loader = None
> -guest.os.loader_ro = None
> -guest.os.loader_type = None
> -guest.os.nvram = None
> -guest.os.nvram_template = None
> +# Implies the default, so clear everything
> +guest.set_legacy_path(None)
> else:
> -# Implies UEFI
> -guest.set_uefi_path(loader)
> -guest.disable_hyperv_for_uefi()
> +if ("bios.bin" or "bios-256k.bin" in loader):
> +# Implies Custom: seabios, not UEFI
> +guest.set_legacy_path(loader)
> +else:
> +# Implies UEFI
> +guest.set_uefi_path(loader)
> +guest.disable_hyperv_for_uefi()
This if/else condition is going to be wrong for all non-x86
architectures.
I wonder if the UI shuld have a "firmware is UEFI" checkbox
instead.
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: [PATCH] Add -m/--machine option. Pick the default machine type depending on arch.
On Thu, Sep 10, 2020 at 01:40:09PM +0100, Richard W.M. Jones wrote: > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1875763 > --- > qemu-sanity-check.in | 20 +--- > qemu-sanity-check.pod.in | 8 > 2 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/qemu-sanity-check.in b/qemu-sanity-check.in > index 5536eda..e1abafd 100644 > --- a/qemu-sanity-check.in > +++ b/qemu-sanity-check.in > @@ -1,7 +1,7 @@ > #!/bin/bash > # -*- shell-script -*- > # qemu-sanity-check > -# Copyright (C) 2013 Red Hat Inc. > +# Copyright (C) 2013-2020 Red Hat Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -27,6 +27,14 @@ canonical_arch="$(uname -m | sed 's/i[456]86/i386/')" > timeout=10m > accel=kvm:tcg > > +# Default machine type depends on arch. You can override this using > +# -m|--machine option. > +case "$canonical_arch" in > +arm*|aarch*) machine=virt ;; > +# On non-ARM let qemu pick the default. > +*) machine= ;; > +esac avr, rx and tricore also lack a defalt but none of those matter for this purpose Reviewed-by: Daniel P. Berrangé 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: [virt-viewer PATCH] about ui: year 2020 in Copyright
On Fri, Aug 28, 2020 at 10:40:07AM -0400, Frediano Ziglio wrote: > > > > On Fri, Aug 28, 2020 at 10:22:49AM -0400, Frediano Ziglio wrote: > > > > > > > > On Thu, Aug 27, 2020 at 09:39:37PM +0300, Uri Lublin wrote: > > > > > rhbz#1848267 > > > > > > > > > > Signed-off-by: Uri Lublin > > > > > > > > Patches need to be subitted as pull requests to the gitlab repo > > > > please. > > > > > > > > > --- > > > > > > > > > > Should po/* files be modified too ? > > > > > > > > Not really. There's no need to keep changing copyright dates in source > > > > files in general. > > > > > > > > > > Yes, but the string this patch refers is marked as translatable and it's > > > translated in some *.po files. It's not about source file dates. > > > > We should make it non-translatable, as it doesn't make sense to change > > copyright statements in translations. > > > > I remember some cases where some people complained that in some countries > the "(C)" was not correct from the legal point of view and needed the usage > of ©. I can't see that as a real issue since every single source file uses "(C)" in license boilerplate and never gets translated. 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: [virt-viewer PATCH] about ui: year 2020 in Copyright
On Fri, Aug 28, 2020 at 10:22:49AM -0400, Frediano Ziglio wrote: > > > > On Thu, Aug 27, 2020 at 09:39:37PM +0300, Uri Lublin wrote: > > > rhbz#1848267 > > > > > > Signed-off-by: Uri Lublin > > > > Patches need to be subitted as pull requests to the gitlab repo > > please. > > > > > --- > > > > > > Should po/* files be modified too ? > > > > Not really. There's no need to keep changing copyright dates in source > > files in general. > > > > Yes, but the string this patch refers is marked as translatable and it's > translated in some *.po files. It's not about source file dates. We should make it non-translatable, as it doesn't make sense to change copyright statements in translations. Regardless, po/*.po files must NEVER be touched by any commit. Only weblate is permitted to touch the po files otherwise we get conflicts. The .pot file is periodically refreshed out of band. > > > > > > > --- > > > src/resources/ui/virt-viewer-about.ui | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/src/resources/ui/virt-viewer-about.ui > > > b/src/resources/ui/virt-viewer-about.ui > > > index 681590f..25b2165 100644 > > > --- a/src/resources/ui/virt-viewer-about.ui > > > +++ b/src/resources/ui/virt-viewer-about.ui > > > @@ -15,7 +15,7 @@ > > > True > > > Virtual Machine Viewer > > > Copyright (C) 2007-2012 > > > Daniel P. Berrange > > > -Copyright (C) 2007-2014 Red Hat, Inc. > > > +Copyright (C) 2007-2020 Red Hat, Inc. > > > A remote desktop client > > > built with GTK-VNC, SPICE-GTK and libvirt > > > > > name="website">https://gitlab.com/virt-viewer/virt-viewer > > > > > > > > name="website_label">https://gitlab.com/virt-viewer/virt-viewer > > > -- > > > 2.26.2 > > > > > > > Regards, > > Daniel 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: [virt-viewer PATCH] about ui: year 2020 in Copyright
On Thu, Aug 27, 2020 at 09:39:37PM +0300, Uri Lublin wrote: > rhbz#1848267 > > Signed-off-by: Uri Lublin Patches need to be subitted as pull requests to the gitlab repo please. > --- > > Should po/* files be modified too ? Not really. There's no need to keep changing copyright dates in source files in general. > > --- > src/resources/ui/virt-viewer-about.ui | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/resources/ui/virt-viewer-about.ui > b/src/resources/ui/virt-viewer-about.ui > index 681590f..25b2165 100644 > --- a/src/resources/ui/virt-viewer-about.ui > +++ b/src/resources/ui/virt-viewer-about.ui > @@ -15,7 +15,7 @@ > True > Virtual Machine Viewer > Copyright (C) 2007-2012 > Daniel P. Berrange > -Copyright (C) 2007-2014 Red Hat, Inc. > +Copyright (C) 2007-2020 Red Hat, Inc. > A remote desktop client > built with GTK-VNC, SPICE-GTK and libvirt > name="website">https://gitlab.com/virt-viewer/virt-viewer > name="website_label">https://gitlab.com/virt-viewer/virt-viewer > -- > 2.26.2 > 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: [PATCH] Correct two translations for zh_CN
On Wed, Aug 05, 2020 at 10:42:22AM -0400, Cole Robinson wrote: > On 8/5/20 4:32 AM, Pavel Hrdina wrote: > > On Wed, Aug 05, 2020 at 12:44:24PM +0800, Huacai Chen wrote: > >> Hi, Pavel, > >> > >> On Mon, Aug 3, 2020 at 8:06 PM Pavel Hrdina wrote: > >>> > >>> On Mon, Aug 03, 2020 at 04:45:03PM +0800, Huacai Chen wrote: > There is no "Error" in "Refresh snapshot list" and "Restoring virtual > machine memory from disk", so remove "出错" in the Chinese translations. > > Signed-off-by: Huacai Chen > --- > po/zh_CN.po | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > >>> > >>> Hi, thank you for your contribution to virt-manager. The translation > >>> changes are not pushed directly to git repository. We use weblate > >>> service hosted by fedora where you should make the changes: > >>> > >>> https://translate.fedoraproject.org/projects/virt-manager/ > >> I have registered, and my user-name is chenhuacai. It seems that I > >> should be a member of cvsl10n, I have joined but it needs to be > >> approved. Could you please help me? > > > > Please keep us informed if you get approved or hit any more roadblocks. > Weblate is new for us so I'm not entirely certain what the process is here > > > I don't have the permissions to approve it but if you follow this guide > > you should be able to get approved https://fedoraproject.org/wiki/L10N/Guide > > > > FWIW Pavel I added you as a virt-manager weblate admin now IMHO, in this kind of case it is better if one of the project maintainers just copy the change into Weblate, instead of expecting the patch submitter to create themselves an account. The account creation is only a good use of time if the contributor intends to become a long term translator. 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: How to update virt-viewer translations
On Thu, Jul 09, 2020 at 04:46:30AM -0400, Frediano Ziglio wrote: > Hi, > I'm trying to fix https://bugzilla.redhat.com/show_bug.cgi?id=1503187, > a translation bug in virt-viewer. > It's not clear how to proceed. Looking at virt-viewer source the translations > are synced from Zanata. I have an account on Zanata but the virt-viewer > project > seems in a read-only state. How to update translations? Zanata is dead, we're moving to Weblate. Needs this reviewed and merged: https://gitlab.com/virt-viewer/virt-viewer/-/merge_requests/14 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: Multiple video elements lead to erratic mouse behavior through spice
On Wed, Jun 10, 2020 at 01:58:13PM +0200, Christian Ehrhardt wrote: > Hi, > with a guest set up as in the following example mouse behavior breaks: > > > > > > > primary='yes'/> > function='0x0'/> > > > > function='0x0'/> > > > Please note that this is for illustration purposes as this everyone can > easily retry. > I originally had QXL + GPU as mediated device, where the behavior is the > same. > > If I know connect to that via spice, spice is smart and opens both displays: > $ virt-viewer --connect qemu+ssh://ubuntu@10.246.115.255/system guestname > > But while with one display mouse integration (to move in/out smoothly) and > positioning of the mouse pointer is fine. > It is broken with this double display setup. > What I see is: > - no mouse integration anymore (falls back to capture the mouse) > - often the mouse can not move upwards anymore > - sometimes "left" also doesn't work > > Let me know if someone wants a video in case the description is still too > odd. > > I have tried spice-client-gtk and virt-viewer, but both behave the same way. > > If I take away one of the displays it works well again. > So I'm wondering how virtual-multi-monitor is supposed to work, are there > known bugs or tweaks that would apply here? Did you enable the SPICE agent ? IIUC, the SPICE agent provides a paravirtualized mouse device, and that may be what's required to have this work properly with multi-monitor. The agent is also what lets you dynamically turn on/off each head and do display resizing. BTW, there are two ways todo multi-monitor with SPICE. One as youve shown here, and the oither using a single QXL card, but with heads=NNN instead of heads=1. One approach works best with Windows, and one approach works best with Linux, but I can't remember which way around. I expect they'll probably still have the same mouse behaviour though. > P.S. I tried VNC but that just has "other" issues. It only shows 1 of 2 > displays and on the one I see over amplifies the mouse movement by the > amount the other display adds to ther overall desktop size. VNC simply doesn't support multi-monitor at all. In theory we could block this in libvirt, but then if QEMU fixed it, libvirt would be needlessly blocking it. 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: Connection issues with virt-viewer 9.0 (win)
On Wed, May 20, 2020 at 12:19:59PM -0500, fletch b wrote: > Hi, > I just upgraded to virt-viever-9.0(win) and ran into issues I never saw > with ver 7,0 > > When trying to connect to my vm I would get to the point where I would see > this: > > "connected to graphic server" and nothing after that other than a spinning > circle. > > It does not do this every time. Out of 20-25 connection attempts, it has > connected just fine 3 times. > > Looking for a pattern and only thing I can come up with is time. If I wait > a few minutes it sometimes will work again. > > Also no sound in 9. > > Gone back to 7 for now.. > any ideas? The lack of sound is reported here and we know the problem and will have a fixed MSI available next week: https://gitlab.com/virt-viewer/virt-viewer/-/issues/2 I've not heard the connectin problem reported before, so please file a bug for that. Also collect the debug logs from spice-gtk or gtk-vnc virt-viewer --debug --spice-debug --gtk-vnc-debug and attach to the new issue 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: How to install an OS with 2 installation iso file
On Tue, May 05, 2020 at 03:14:25PM +0100, Richard W.M. Jones wrote: > On Sun, May 03, 2020 at 03:31:38PM +0530, Harsh chopra wrote: > > Hi everyone, > > I was trying to install Centos 2.1 on the virtual machine, > > Ambitious! Note that older operating systems often don't work in > qemu, not for any particular reason but just because they don't get > much attention and things regress, drivers in particular. Or the > drivers such as IDE don't emulate corners which are needed by the old > OS. For example I tried running some really ancient Linux versions in > qemu a while back with no success at all. > > Having said that I have run RHEL 3 recently on qemu and that's fairly > similar to 2.1. > > > the installation > > was complete 50% but then it asked to insert the second installation iso > > file. > > i.e. "Please insert disc 2 in cdrom" > > I tried to change the XML file and insert the line > > > > under device name but nothing helped. > > When you say you "tried to change the XML file" - how did you do that? > You should use ‘virsh edit ’ only, and not attempt to edit any > XML file directly. Or of course you can use virt-manager as the other > reply suggested - it essentially does the same thing as virsh edit. Actually "virsh edit" is not what you need in this case. That will update the persistent XML config only. For a multi-stage installer, you need to perform a CD media change in the live VM. IIRC the following should work: $ virsh change-media $GUESTNAME /dev/hdc --update /path/to/iso This is equivalent to using "--eject" followed by "--insert /path/to/iso" 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 :|
ANNOUCE: virt-viewer version 9.0 released
I am happy to announce a new bugfix release of virt-viewer 9.0: https://virt-manager.org/download/sources/virt-viewer/virt-viewer-9.0.tar.gz https://virt-manager.org/download/sources/virt-viewer/virt-viewer-9.0.tar.gz.asc including experimental Windows installers for Win x86: https://virt-manager.org/download/sources/virt-viewer/virt-viewer-x86-9.0.msi https://virt-manager.org/download/sources/virt-viewer/virt-viewer-x86-9.0.msi.asc and Win x64: https://virt-manager.org/download/sources/virt-viewer/virt-viewer-x64-9.0.msi https://virt-manager.org/download/sources/virt-viewer/virt-viewer-x64-9.0.msi.asc Signatures are created with key DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF (4096R) With this release the project has moved over to use GitLab for its hosting needs instead of Pagure. Instead of sending patches to the old mailing list, we have adopted modern best practices and now welcome contributions as merge requests, from where they undergo automated CI testing of the build. Bug reports, directed towards upstream maintainers, should also be filed at the GitLab project now instead of the Red Hat Bugzilla. All historical releases are available from: http://virt-manager.org/download/ Changes in this release include: - Project moved to https://gitlab.com/virt-viewer/virt-viewer - Allow toggling shared clipboard in remote-viewer - Fix handling when initial spice connection fails - Fix check for govirt library - Add bash completion of cli args - Improve errors in file transfer dialog - Fix ovirt foreign menu storage domains query - Prefer TLS certs from oVirt instead of CLI - Improve USB device cleanup when Ctrl-C is used - Remember monitor mappings across restarts - Add a default file extension to screenshots - Updated translations - Fix misc memory leaks 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: [PATCH] virtinst: use 'host-passthrough' as default 'host' on AArch64
On Tue, Apr 07, 2020 at 08:01:50AM -0400, Liang Yan wrote: > Thanks for the input. > > On 4/7/20 3:54 AM, Andrea Bolognani wrote: > > On Mon, 2020-04-06 at 16:51 -0400, Liang Yan wrote: > >> virt-install already uses 'host-passthrough' as default when no setup > >> for cmline '--cpu'. However, it will still use 'host-model' when comes > >> with '-cpu host'. This will be a problem for aarch64 platfrom as > >> 'host-model' for aarch64 kvm domain on aarch64 host is not supported yet. > > > > virt-install's --cpu host maps to libvirt's > > > > > > > > while virt-install's --cpu host-passthrough maps to libvirt's > > > > > > > > These semantics are not architecture dependent and, while the choice > > of name is a bit unfortunate considering that QEMU's -cpu host and > > virt-install's --cpu host have different meanings, I think they're > > completely unambiguous and reasonably documented, and we should not > > mess with them. > > > > > virt-install supports both "host","host-mdoel" and "host-passthrough", > "--cpu host" is mapped to "host-model" based on "cli:det_model_b", > however if you check the default setup in "cpu"set_defaults", it has > different configuration. Btw, it has different setup for different > architecture already. Nova also has different setup for aarch64 in my > defense. > > https://opendev.org/openstack/nova/commit/8bc7b950b7c0a3c80cdd120fe4df97c14848c344 > > > > I agree we could document this situation at least. It does block our > aarch64 tests while it is ok for x86_64. That is a bug in the tests - they shouldn't be trying to use a feature which doesn't make sense for that arch, 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 :|
virt-viewer move to GitLab, with merge requests
Hi All, Historically the virt-viewer project has accepted contributions via patches submitted to this mailing list, with the master git repository originally being on fedorahosted.org and then on pagure.io. With libvirt moving to GitLab and merge requests, and spice-gtk/gtk-vnc also both already being on GitLab, it is time to move virt-viewer too. Henceforth the master repository is now: https://gitlab.com/virt-viewer/virt-viewer/ We now expect patches to be submitted as merge requests to this repo, and so use of this mailing list should be discontinued for work related to virt-viewer. Similar use of bugzilla.redhat.com for *upstream* issues should also stop, in favour of the gitlab issue tracker. I also intend to stop using virt-manager.org for release news, downloads, etc, since virt-viewer has nothing in common with virt-manager except for the fact that both projects were originally started by me. It is increasingly confusing / unhelpful to use the same website for both projects. I'll probably just setup a simple static site on gitlab pages. 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: [PATCH remote-viewer] Fix potential file descriptor leaks found by Coverity.
On Wed, Apr 01, 2020 at 09:25:18AM +0200, Julien Ropé wrote: > The error code returned by virt_viewer_session_open_fd() and > virt_viewer_session_channel_open_fd() were not checked. The file descriptor > passed to them could then be left opened even if the function failed, causing > a leak of resources. > > This was reported by a Coverity scan, logged under > https://bugzilla.redhat.com/show_bug.cgi?id=1655792 > > Signed-off-by: Julien Ropé > --- > src/virt-viewer-app.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé 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: Adding an "Enable Launch Security" checkbox to the Memory Details dialog
On Fri, Mar 27, 2020 at 12:13:09PM -0400, Cole Robinson wrote: > CCing Erik who knows more about that launchSecurity/sev than I do > > On 3/27/20 11:44 AM, Charles Arnold wrote: > > What is the opinion of adding a checkbox called "Enable Launch > > Security" under the 'Current allocation' and 'Maximum allocation' boxes > > on the Details->Memory dialog? It would only be enabled if libvirt > > detected support for it. > > > > Provided libvirt capabilities report everything we need to know to > whether it's really supported on the host and will actually work, and > there's a sensible noncontroversial set of defaults we can fill in, then > a single checkbox is worth considering. It's certainly an advanced > feature but it's also getting more and more mention these days so maybe > it's good to get out ahead of any future RFEs. Two issues right now. There is a ridiculously low limit of 15 VMs on first generation CPUs, perhaps not a huge problem for typical scenarios using virt-manager though. Second though is that while libvirt reports whether the feature exists & is supported in QEMU, QEMU is lieing to us, because it isn't checking whether kvm-amd actually allows the feature to be used. https://bugzilla.redhat.com/show_bug.cgi?id=1689202 https://bugzilla.redhat.com/show_bug.cgi?id=1731439 As long as the checkbox isn't enabled by default, its probably ok to ignore those two issues 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: shm as domain to virt-viewer protocol?
On Wed, Mar 25, 2020 at 06:16:47PM +, David Geise wrote: > Hi, > > I've been using an open-source project that's currently in beta called > Looking Glass (https://looking-glass.hostfission.com/) to achieve high- > performance VM graphics in a standalone workstation scenario where the > guest VM and viewer are both on the host. I use it to virtualize my > windows development environment. I need it to do work in the Unreal > Engine, which requires full GPU acceleration. My understanding is that > currently GPU isn't supported directly in the virt-manager/virt-viewer > environment. I know the cloud is the big thing these days but this > windows vm-on-host scenario is an increasingly common use-case and it > seems like there's a great opportunity to improve virt-viewer/virt-manager > here. I expect if Linux were to better support a high-performance windows+ > gpu on linux scenario, this should improve consumer pc uptake of linux. This isn't my area of expertise, but my understanding was/is that the virtio-gpu device and spice are intended to fill the GPU accelerated rendering scenario. IIUC this will include shared memory buffers between guest and host QEMU and host virt-viewer (well spice-gtk), to avoid memory copies in the local scenario. I think the main difference is probably the guest focus - virtio-gpu has targetted OpenGL accleration IIUC with plans for Vulkan too, which are the common technologies for Linux, where as looking glass is presuambly focusing on Windows rendering, though honestly this is mostly guesswork on my part. > With all that being said, I'd like to ask the virt-viewer developers: > > 1. Is there any interest in developing a low-latency 'standalone' transport > for virt-viewer? Is any such work contemplated or underway? Or is some > alternative (virtio-gpu comes to mind) being worked on that is an even > better alternative? Yeah, my hope/expectation was that virtio-gpu / spice-gtk are going to solve this problem. From the virt-viewer side, we really leave all the real work to the spice-gtk client widget. virt-viewer just adds the window management on top, so we don't really get involved in low level stuff that's performance critical ourselves. > 2. Is there truly an intrinsic opposition to, say, a proposed Gtk-shm > widget & corresponding driver being added to the virt viewer project? > Would it be possible / non-problematic to develop a platform-neutral > solution to optimize latency for this local vm scenario? As you know virt-viewer/virt-manager currently have VNC and SPICE support, with majority of important work taking place in context of SPICE. It is certainly possible to add further backends, but that will obviously have a cost associated with it. Assuming the development work taking place in QEMU and spice to support accelerated graphics is going to deliver a viable solution, then I'd not be too enthusiastic about adding an alternative backend that tries to solve the same problem. I think is more compelling to users to have a single solution that can deliver everything they need rather than having to decide between many choices. > I'm contemplating whether to try to fork virt-viewer and do a shm transport > integration myself or if I should contribute to the looking-glass project > instead. Would the virt-developers like to participate or take the lead > in this? Would the repository owners be open to a pull request to such a > contribution? Would RedHat sign such a driver? I expect there might be > strategic / political implications at play here. I think perhaps it might be worth taking your message to the spice mailing list for feedback, as they can probably give a better viewpoint on the real technical details for how looking-glass project compares to the work that is being done for virtio-gpu / spice than I can, and thus whether there's a need for both solutions or better to focus on one only. > For what it's worth, I've got 25 years of multi-language experience, mostly > heavy C++, mostly on windows. My C / DDK experience is rusty (like 15 years > since I've coded in raw C, even longer since unix!) but I might be able to > contribute something worthwhile. One thing that's true for all open source projects is that there is not enough developer time todo everything they want, so whether its working on looking-glass vs spice/virtio-gpu, I'm sure there's useful work that can be contributed. 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: [PATCH remote-viewer v2 1/1] Remember monitor mapping on close.
On Fri, Feb 07, 2020 at 10:14:57AM +0100, Julien Ropé wrote: > When the application is stopped, if the windows are in fullscreen, their > position on the client will be remembered. > > This change uses the existing option 'monitor-mapping' in the settings > file to save the position and reuse it on next launch. > > This implements part of the requirement from > https://bugzilla.redhat.com/show_bug.cgi?id=1179070 > > NOTE: this feature is effective only with GTK >= 3.22 > > Signed-off-by: Julien Ropé > --- > src/virt-viewer-app.c | 120 ++ > 1 file changed, 120 insertions(+) Reviewed-by: Daniel P. Berrangé and pushed to git master 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: [PATCH remote-viewer 1/1] Remember monitor mapping on close.
On Wed, Feb 05, 2020 at 10:02:25AM +0100, Julien ROPE wrote: > > Hi Daniel, > > I looked into this, and I'm not sure how to fix it. > > I'm new to GTK. But from what I can tell, there was no way of accessing > Monitor information before GTK 3.22. > > All those APIs that I'm using were introduced in this version, and in > previous documentation on GTK, I find no equivalent. > > If any of you know differently, I'd be happy to be corrected. > > > If that's the case, it means we can't add this feature on platforms with GTK > < 3.22. > > Now I find code here and there in virt-viewer that are protected with "#if > GTK_CHECK_VERSION(...)" statements (cf. virt_viewer_window_enter_fullscreen() > for instance). > > Should I use something like that to make that code available only with GTK > 3.22 and above? Yes, you can use conditionals like that. You'll need to squash deprecation warnings as that code does too. 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: [PATCH remote-viewer 1/1] Remember monitor mapping on close.
On Tue, Jan 21, 2020 at 08:48:35AM +0100, Julien Ropé wrote:
> When the application is stopped, if the windows are in fullscreen, their
> position on the client will be remembered.
>
> This change uses the existing option 'monitor-mapping' in the settings
> file to save the position and reuse it on next launch.
>
> This implements part of the requirement from
> https://bugzilla.redhat.com/show_bug.cgi?id=1179070
>
> Signed-off-by: Julien Ropé
> ---
> src/virt-viewer-app.c | 114 ++
> 1 file changed, 114 insertions(+)
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index da8cfa9..f955882 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -106,6 +106,7 @@ static void virt_viewer_app_set_fullscreen(VirtViewerApp
> *self, gboolean fullscr
> static void virt_viewer_app_update_menu_displays(VirtViewerApp *self);
> static void virt_viewer_update_smartcard_accels(VirtViewerApp *self);
> static void virt_viewer_app_add_option_entries(VirtViewerApp *self,
> GOptionContext *context, GOptionGroup *group);
> +static VirtViewerWindow *virt_viewer_app_get_nth_window(VirtViewerApp *self,
> gint nth);
>
>
> struct _VirtViewerAppPrivate {
> @@ -400,6 +401,116 @@
> virt_viewer_app_get_monitor_mapping_for_section(VirtViewerApp *self, const
> gchar
> return mapping;
> }
>
> +/*
> + * save the association display/monitor in the config for reuse on next
> connection
> + */
> +static void virt_viewer_app_set_monitor_mapping_for_display(VirtViewerApp
> *self, VirtViewerDisplay *display)
> +{
> +GError *error = NULL;
> +gsize nmappings = 0;
> +gchar **mappings = NULL;
> +gchar **tokens = NULL;
> +
> +int i;
> +
> +gint virt_viewer_display = virt_viewer_display_get_nth(display);
> +gint virt_viewer_monitor = virt_viewer_display_get_monitor(display);
> +
> +if (virt_viewer_monitor == -1) {
> +// find which monitor the window is on
> +GdkDisplay *gdk_dpy = gdk_display_get_default();
> +VirtViewerWindow *vvWindow = virt_viewer_app_get_nth_window(self,
> virt_viewer_display) ;
> +GdkWindow *window =
> gtk_widget_get_window(GTK_WIDGET(virt_viewer_window_get_window(vvWindow)));
> +GdkMonitor *pMonitor = gdk_display_get_monitor_at_window(gdk_dpy,
> window);
This breaks the build due to using APIs that are newer than our min
reqiured GTK version:
virt-viewer-app.c: In function
'virt_viewer_app_set_monitor_mapping_for_display':
virt-viewer-app.c:428:9: warning: 'gdk_display_get_monitor_at_window' is
deprecated: Not available before 3.22 [-Wdeprecated-declarations]
428 | GdkMonitor *pMonitor =
gdk_display_get_monitor_at_window(gdk_dpy, window);
| ^~
In file included from /usr/include/gtk-3.0/gdk/gdkscreen.h:32,
from /usr/include/gtk-3.0/gdk/gdkapplaunchcontext.h:31,
from /usr/include/gtk-3.0/gdk/gdk.h:32,
from /usr/include/gtk-3.0/gtk/gtk.h:30,
from virt-viewer-app.c:28:
/usr/include/gtk-3.0/gdk/gdkdisplay.h:194:14: note: declared here
194 | GdkMonitor * gdk_display_get_monitor_at_window (GdkDisplay *display,
| ^
We currently request
GTK_REQUIRED="3.12"
GTK_ENCODED_VERSION="GDK_VERSION_3_12"
based on our CI platforms, Ubuntu 16.04 is the oldest GTK
at version 3.18.9
So we can bump our min GTK to 3.18, but we can't use
APIs from 3.22 yet
Once we drop Ubuntu 16.04 - most likely around the April 2020 timefram
when Ubuntu 20.04 is released, then Debian 9 & CentOS 7 will be the
oldest at version 3.22.
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: virt-manager version question
On Mon, Jan 27, 2020 at 02:15:28PM -0500, Scott Reeve wrote: > Virtual Machine Manager version I have is 0.9.5. > Says: Copyright (C) 2006-2011 Red Hat Inc. > > I see this version on a ubuntu machine running 14.04 (yes old). > See the same version on a Centos machine running 6.6. > > On a Centos 7 machine, I get virt-manager 1.5. > > Is it possible to run the latest virt-manager on ubuntu 14.04 or Centos 6.6 I don't know what your constraints are wrt the operating systems you have to use, but if at all possible, I'd recommend installing a much more modern OS. The virtualization world has massively advanced in the time since Ubuntu 14.04 / Centos 6.6. 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: [virt-viewer PATCH v4 1/1] remote-viewer: add handler for SIGINT signal
On Fri, Jan 17, 2020 at 04:06:13PM +0100, Francesco Giudici wrote: > When remote-viewer is started from terminal, CTRL-C sends a SIGINT > signal to the program causing immediate termination. On linux clients > usb redirected devices are left without any kernel driver attached, > causing them to appear as no more available to the host. > Add a SIGINT handler to allow a clean exit, in particular to allow > the kernel to attach back the host driver. > The issue is present on linux only. > > To perform usb device redirection, virt-viewer leverages spice-gtk > library, which in turn relies on usbredir library, which uses libusb. > In order to take control of the usb device the auto-loaded kernel > driver must be detached. This is achieved (in the very end) with > libusb_detach_kernel_driver(). Then the device interfaces can be claimed > with libusb_claim_interface() and get in control to the application. > During normal application termination, the usb channel is teared down, > performing a reset of the usb device and giving back the control of the > device to the kernel (libusb_attach_kernel_driver()). > If the application quits without doing this, the device interfaces will > end up with no driver attached, making them not usable in the host > system. > > Note that enabling libusb_set_auto_detach_kernel_driver() does not solve > the issue, as this is just a convenient API from libusb: libusb will > take care of detaching/attaching the driver to the interfaces of the usb > device each time a call to libusb_release_interface() and > libusb_claim_interface() is performed. An unexpected quit of the > application will skip the libusb_release_interface() call too, leaving > the interfaces without any driver attached. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1713311 > > Signed-off-by: Francesco Giudici > --- > src/virt-viewer-app.c | 25 + > 1 file changed, 25 insertions(+) Thanks, this is merged 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: [virt-viewer PATCH v2] remote-viewer: add handler for SIGINT signal
On Fri, Jan 17, 2020 at 12:04:08PM +0100, Francesco Giudici wrote:
> When remote-viewer is started from terminal, CTRL-C sends a SIGINT
> signal to the program causing immediate termination. On linux clients
> usb redirected devices are left without any kernel driver attached,
> causing them to appear as no more available to the host.
> Add a SIGINT handler to allow a clean exit, in particular to allow
> the kernel to attach back the host driver.
> The issue is present on linux only.
>
> To perform usb device redirection, virt-viewer leverages spice-gtk
> library, which in turn relies on usbredir library, which uses libusb.
> In order to take control of the usb device the auto-loaded kernel
> driver must be detached. This is achieved (in the very end) with
> libusb_detach_kernel_driver(). Then the device interfaces can be claimed
> with libusb_claim_interface() and get in control to the application.
> During normal application termination, the usb channel is teared down,
> performing a reset of the usb device and giving back the control of the
> device to the kernel (libusb_attach_kernel_driver()).
> If the application quits without doing this, the device interfaces will
> end up with no driver attached, making them not usable in the host
> system.
>
> Note that enabling libusb_set_auto_detach_kernel_driver() does not solve
> the issue, as this is just a convenient API from libusb: libusb will
> take care of detaching/attaching the driver to the interfaces of the usb
> device each time a call to libusb_release_interface() and
> libusb_claim_interface() is performed. An unexpected quit of the
> application will skip the libusb_release_interface() call too, leaving
> the interfaces without any driver attached.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1713311
>
> Signed-off-by: Francesco Giudici
> ---
> src/virt-viewer-app.c | 79 +++
> 1 file changed, 79 insertions(+)
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index da8cfa9..06e237b 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -36,6 +36,7 @@
> #include
> #include
> #include
> +#include
>
> #ifdef HAVE_SYS_SOCKET_H
> #include
> @@ -1756,6 +1757,74 @@ static gboolean opt_fullscreen = FALSE;
> static gboolean opt_kiosk = FALSE;
> static gboolean opt_kiosk_quit = FALSE;
>
> +#ifndef G_OS_WIN32
> +static int sigint_pipe[2];
> +
> +static void
> +sigint_handler(int signum)
> +{
> +int savedErrno;
> +
> +g_return_if_fail(signum == SIGINT);
> +
> +savedErrno = errno;
> +if (write(sigint_pipe[1], "x", 1) == -1 && errno != EAGAIN)
> +g_debug("SIGINT handler failure\n");
> +errno = savedErrno;
> +}
> +
> +static void
> +register_sigint_handler()
> +{
> +int flags, i;
> +struct sigaction sa;
> +
> +if (pipe(sigint_pipe) == -1)
> +goto err;
> +
> +for (i = 0; i < 2; i++) {
> +flags = fcntl(sigint_pipe[i], F_GETFL);
> +if (flags == -1)
> +goto err;
> +flags |= O_NONBLOCK;
> +if (fcntl(sigint_pipe[i], F_SETFL, flags) == -1)
> +goto err;
> +}
> +
> +sigemptyset(_mask);
> +sa.sa_flags = SA_RESTART;
> +sa.sa_handler = sigint_handler;
> +if (sigaction(SIGINT, , NULL) == -1)
> +goto err;
> +
> +return;
> +
> +err:
> +g_debug("Cannot register SIGINT handler\n");
> +}
> +
> +static gboolean
> +sigint_cb(GIOChannel *source,
> + GIOCondition condition,
> + gpointer data)
> +{
> +VirtViewerApp *self = VIRT_VIEWER_APP(data);
> +VirtViewerAppPrivate *priv = self->priv;
> +gchar sbuf;
> +
> +g_assert(condition == G_IO_IN);
> +
> +g_debug("got SIGINT, quitting\n");
> +if (priv->started)
> +virt_viewer_app_quit(self);
> +else
> +exit(0);
> +
> +g_io_channel_read_chars (source, , 1, NULL, NULL);
> +return TRUE;
> +}
> +#endif
> +
> static void
> title_maybe_changed(VirtViewerApp *self, GParamSpec* pspec G_GNUC_UNUSED,
> gpointer user_data G_GNUC_UNUSED)
> {
> @@ -1766,10 +1835,20 @@ static void
> virt_viewer_app_init(VirtViewerApp *self)
> {
> GError *error = NULL;
> +#ifndef G_OS_WIN32
> +GIOChannel *sigint_channel = NULL;
> +#endif
> +
> self->priv = virt_viewer_app_get_instance_private(self);
>
> gtk_window_set_default_icon_name("virt-viewer");
>
> +#ifndef G_OS_WIN32
> +register_sigint_handler();
> +sigint_channel = g_io_channel_unix_new(sigint_pipe[0]);
> +g_io_add_watch(sigint_channel, G_IO_IN, sigint_cb, self);
> +#endif
Just yesterday I learnt that GLib has native support for signal handling
in its event loop which allows this to be simplified:
g_unix_signal_add (SIGINT, sigint_cb, self);
> self->priv->displays = g_hash_table_new_full(g_direct_hash,
> g_direct_equal, NULL, g_object_unref);
> self->priv->config = g_key_file_new();
> self->priv->config_file =
Re: [virt-tools-list] FYI: intention to remove mail subject prefix & footer text
FYI, I'm intending to make this change tomorrow (Thurs Jan 16th) On Fri, Jan 10, 2020 at 11:44:04AM +, Daniel P. Berrangé wrote: > Hi List Subscribers, > > In recent months we have been seeing an increasing number of bounced > deliveries from the mailing list due to DMARC policies on list > subscriber's mail servers. IOW, many subscribers are only receiving > a subset of mails sent to this list. > > We believe the root cause of many of the problems is that mailman is > modifying the mail subject to add the "[virt-tools-list]" prefix, and > modifying the mail body to add the footer with links to the listinfo > page. > > These modifications invalidate the DKIM signatures on mails sent to > the list by some of our subscribers. This in turn causes DMARC policy > rejections by the destination SMTP servers when mailman delivers > messages. > > The solution is to disable any feature in mailman which modifies > parts of the mail validated by the DKIM signature. This means removing > the subject prefix and the mail body header. Further information on > this approach can be seen here: > >https://begriffs.com/posts/2018-09-18-dmarc-mailing-list.html > > QEMU has made the same change on their mailing list last year: > >https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00416.html > > If you are currently doing mail filtering / sorting based on the subject > prefix, you will need to change to use the List-Id header instead. > > I will wait until the latter part of next week before making this change > to allow people time to adapt any filters. > > 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 :| > > ___ > virt-tools-list mailing list > virt-tools-list@redhat.com > https://www.redhat.com/mailman/listinfo/virt-tools-list 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] FYI: intention to remove mail subject prefix & footer text
Hi List Subscribers, In recent months we have been seeing an increasing number of bounced deliveries from the mailing list due to DMARC policies on list subscriber's mail servers. IOW, many subscribers are only receiving a subset of mails sent to this list. We believe the root cause of many of the problems is that mailman is modifying the mail subject to add the "[virt-tools-list]" prefix, and modifying the mail body to add the footer with links to the listinfo page. These modifications invalidate the DKIM signatures on mails sent to the list by some of our subscribers. This in turn causes DMARC policy rejections by the destination SMTP servers when mailman delivers messages. The solution is to disable any feature in mailman which modifies parts of the mail validated by the DKIM signature. This means removing the subject prefix and the mail body header. Further information on this approach can be seen here: https://begriffs.com/posts/2018-09-18-dmarc-mailing-list.html QEMU has made the same change on their mailing list last year: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00416.html If you are currently doing mail filtering / sorting based on the subject prefix, you will need to change to use the List-Id header instead. I will wait until the latter part of next week before making this change to allow people time to adapt any filters. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-manager: snapshot uefi vm
On Mon, Dec 23, 2019 at 03:27:05PM -0800, femi wrote: > When will snapshots of UEFI virtual machines become a reality? It is blocked by limitations in QEMU. The savevm command doens't have any way to be told a list of disks to snapshot & this breaks with UEFI as it considers the NVRAM file a disk to snapshot. So savevm needs rewriting in QEMU, and then wiring up in libvirt before it can be done in virt-manager. Unfortunately no one in QEMU seems to care about internal snapshots, only external snapshots, so there's no ETA :-( 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-manager test suite hangs on FreeBSD
On Thu, Dec 12, 2019 at 09:47:42AM +0100, Andrea Bolognani wrote: > On Wed, 2019-12-11 at 18:01 -0500, Cole Robinson wrote: > > On 12/11/19 7:40 AM, Andrea Bolognani wrote: > > > On Wed, 2019-12-11 at 07:28 -0500, Cole Robinson wrote: > > > > On 12/11/19 5:22 AM, Andrea Bolognani wrote: > > > > > I have no idea how to debug this thing. Can an actual virt-manager > > > > > developer jump in? > > > > I reproduced. Part of the test suite was forking off /bin/true to hit a > > code path. But when /bin/true wasn't available it would leave a stranded > > process which caused the hang. Fixed both now upstream > > > > > > You can > > > > use './setup.py test --debug' which may give more info where it is > > > > hanging. Possibly somewhere in the cli tests where we try to handle mock > > > > stdin or try to fake --wait timeouts > > > > > > Doing so doesn't really result in additional information being > > > printed out: it still just quietly hangs there. > > > > This was a separate bug, calling into the cli tools for clitest.py would > > reset the test suite debugging. That's fixed too > > That's awesome, thank you so much! I already posted a patch[1] that > enables the test suite on FreeBSD in the CentOS CI environment, so > if a regression manages to sneak in we'll catch it right away. > > One more thing. Right now virt-manager is the only project we have on > CentOS CI that uses the python3-libxml2 bindings in the first place, > with all others using python3-lxml instead, and that leads to a > reduced test matrix for it because the former are not available on > Ubuntu 16.04 and CentOS 7. > > So that makes me wonder, does python3-lxml provide a better API or > something? Would it be a massive amount of work to port virt-manager > to it, and would it even make sense to do so? In due time we'll stop > supporting those two targets anyway... They both use libxml native library under the hood. The lxml bindings are exposing libxml in a way that is "more pythonic" and thus easier and safer to use. So in long term I expect virt-manager would likely benefit from lxml, if anyone wants to do the grunt work. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Mon, Nov 25, 2019 at 11:29:38AM +, Richard W.M. Jones wrote: > On Mon, Nov 25, 2019 at 11:23:14AM +0000, Daniel P. Berrangé wrote: > > On Mon, Nov 25, 2019 at 11:13:00AM +, Richard W.M. Jones wrote: > > > On Fri, Nov 22, 2019 at 09:56:17AM +, Daniel P. Berrangé wrote: > > > > On Thu, Nov 21, 2019 at 09:39:46PM -0500, Dusty Mabe wrote: > > > > > > > > > > > > > > > On 11/21/19 6:20 AM, Daniel P. Berrangé wrote: > > > > > > On Thu, Nov 21, 2019 at 11:07:24AM +0000, Richard W.M. Jones wrote: > > > > > >> On Thu, Nov 21, 2019 at 10:34:14AM +, Daniel P. Berrangé wrote: > > > > > >>> On Wed, Nov 20, 2019 at 08:18:01PM -0500, Dusty Mabe wrote: > > > > > >>>> Basically in Fedora CoreOS we need a generic user data mechanism > > > > > >>>> that works across > > > > > >>>> platforms (x86_64, aarch64, ppc64le, s390x) and doesn't have > > > > > >>>> possible race conditions. > > > > > >>>> Right now we're using `-fw_cfg` but it's not cross platform. We > > > > > >>>> don't have an answer > > > > > >>>> yet: > > > > > >>>> https://github.com/coreos/ignition/issues/666#issuecomment-452835654 > > > > > >>> > > > > > >>> For platform portability you need to find some device that is > > > > > >>> common > > > > > >>> across all platforms, and either disk or network are the only two > > > > > >>> good options that exist today or for the forseeable future. If > > > > > >>> those > > > > > >>> aren't acceptable then all we have left are platform specific > > > > > >>> options. > > > > > >> > > > > > >> While it's not a "good option that exists today", AF_VSOCK would > > > > > >> be a > > > > > >> good choice to settle on in the future. It's completely cross > > > > > >> platform, available for Windows, and doesn't interfere with > > > > > >> existing > > > > > >> network or disk devices. > > > > > >> > > > > > >> Would needing virtio be a barrier? Our impl of AF_VSOCK runs over > > > > > >> virtio, but there are other transports. > > > > > > > > > > > > From a cloud-init POV, I don't see virtio as a barrier. Defining an > > > > > > AF_VSOCK data source for it should be quite straightforward really > > > > > > and they already have so many data sources, it seems reasonable > > > > > > that they'd accept one more. > > > > > > > > > > > > On the host side there's still the task of providing a metdata > > > > > > service to expose the data, which is outside scope of virt-install. > > > > > > > > > > > > > > > > Thanks for the fruitful conversation here regarding a cross platform > > > > > data source > > > > > that we could use. Is this worth us writing up into a request for an > > > > > issue tracker > > > > > somewhere where it could be discussed further? > > > > > > > > Possibly - depends if AF_VSOCK is viable or not for Ignition. > > > > > > > > Previously when we suggested use of a plain disk for Ignition, the issue > > > > raised was that the /dev/sdXXX device nodes take a non-deterministic > > > > amount of time to appear, and ignition doesn't know how long to wait > > > > for them, because the disks might not exist at all in the first place. > > > > > > > > Using AF_VSOCK is going to have exactly this same problem, so personally > > > > I can't see it can satisfy Ignition, where virtio-blk failed. > > > > > > Be interesting what exactly their concerns were. Obviously both PCI > > > initialization and (especially) kernel disk enumeration can take a > > > long time with guests that have a lot of PCI slots or disks. But > > > that's what udev is for, right :-) Couldn't they just hook up their > > > initialization to a udev rule? > > > > I mention it above - they don't know whether the user has even > > provided the metadata in the first place. They don't want to wait > > for a dev
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Mon, Nov 25, 2019 at 11:13:00AM +, Richard W.M. Jones wrote: > On Fri, Nov 22, 2019 at 09:56:17AM +0000, Daniel P. Berrangé wrote: > > On Thu, Nov 21, 2019 at 09:39:46PM -0500, Dusty Mabe wrote: > > > > > > > > > On 11/21/19 6:20 AM, Daniel P. Berrangé wrote: > > > > On Thu, Nov 21, 2019 at 11:07:24AM +, Richard W.M. Jones wrote: > > > >> On Thu, Nov 21, 2019 at 10:34:14AM +, Daniel P. Berrangé wrote: > > > >>> On Wed, Nov 20, 2019 at 08:18:01PM -0500, Dusty Mabe wrote: > > > >>>> Basically in Fedora CoreOS we need a generic user data mechanism > > > >>>> that works across > > > >>>> platforms (x86_64, aarch64, ppc64le, s390x) and doesn't have > > > >>>> possible race conditions. > > > >>>> Right now we're using `-fw_cfg` but it's not cross platform. We > > > >>>> don't have an answer > > > >>>> yet: > > > >>>> https://github.com/coreos/ignition/issues/666#issuecomment-452835654 > > > >>> > > > >>> For platform portability you need to find some device that is common > > > >>> across all platforms, and either disk or network are the only two > > > >>> good options that exist today or for the forseeable future. If those > > > >>> aren't acceptable then all we have left are platform specific options. > > > >> > > > >> While it's not a "good option that exists today", AF_VSOCK would be a > > > >> good choice to settle on in the future. It's completely cross > > > >> platform, available for Windows, and doesn't interfere with existing > > > >> network or disk devices. > > > >> > > > >> Would needing virtio be a barrier? Our impl of AF_VSOCK runs over > > > >> virtio, but there are other transports. > > > > > > > > From a cloud-init POV, I don't see virtio as a barrier. Defining an > > > > AF_VSOCK data source for it should be quite straightforward really > > > > and they already have so many data sources, it seems reasonable > > > > that they'd accept one more. > > > > > > > > On the host side there's still the task of providing a metdata > > > > service to expose the data, which is outside scope of virt-install. > > > > > > > > > > Thanks for the fruitful conversation here regarding a cross platform data > > > source > > > that we could use. Is this worth us writing up into a request for an > > > issue tracker > > > somewhere where it could be discussed further? > > > > Possibly - depends if AF_VSOCK is viable or not for Ignition. > > > > Previously when we suggested use of a plain disk for Ignition, the issue > > raised was that the /dev/sdXXX device nodes take a non-deterministic > > amount of time to appear, and ignition doesn't know how long to wait > > for them, because the disks might not exist at all in the first place. > > > > Using AF_VSOCK is going to have exactly this same problem, so personally > > I can't see it can satisfy Ignition, where virtio-blk failed. > > Be interesting what exactly their concerns were. Obviously both PCI > initialization and (especially) kernel disk enumeration can take a > long time with guests that have a lot of PCI slots or disks. But > that's what udev is for, right :-) Couldn't they just hook up their > initialization to a udev rule? I mention it above - they don't know whether the user has even provided the metadata in the first place. They don't want to wait for a device that doesn't exist or they'll be waiting forever. If you wait with a timeout, then it delays boot, and creates failure conditions under high load. > > The main benefit I see of AF_VSOCK over virtio-blk disk / cdrom, is that > > it is a live data channel so you are not restricted to providing data that > > is fixed at boot time, it can be live updated on the fly. > > > > The only mechanism that can avoid the problem of waiting for some > > device driver to initialize asynchonrously is to use a directly > > mapped memory region that is immediately exposed by the core kenrel > > functionality. This is what fw_cfg, SMBIOS, and/or kernel command > > line args all achieve, and the other options not based on mapped > > memory will all fail to achieve. > > > > Regards, > > Daniel > > I added Stefan to CC because he might be interested in this use case too. > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > Fedora Windows cross-compiler. Compile Windows programs, test, and > build Windows installers. Over 100 libraries supported. > http://fedoraproject.org/wiki/MinGW 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Thu, Nov 21, 2019 at 07:44:38PM -0500, Cole Robinson wrote: > On 11/21/19 5:28 AM, Daniel P. Berrangé wrote: > > One option is to take the lazy approach and always enable cloud init > > if we are given a pre-built disk image. > > > > * If the disk image doesn't support cloud init, then its fairly > >harmless to provide cloud-init data. > > > > * If the disk image does support cloud init but has already been > >booted, and the user has marked cloud init as disable, then it > >is again harmless to provide cloud-init data > > > > * If the disk image does support cloud init but has already been > >booted, and the user left cloud init enabled so that it runs on > >every boot, then passing cloud init data is probably good thing. > > > > * If the disk image does support cloud init and has never been > >booted, then obviously providing cloud init data is good > > > > If we had a SMBIOS or -fw_cfg option, then maybe this is an option. But > I don't think I could stomach doing this by default with the current > 'cdrom' approach, surely there would be complications, and at minimum > complaints about a needless attached CDROM device. Yeah, I can understand this concern. > I think the 'cdrom' case is good enough for a first crack at this and > anything else can build on that initial support. I think the command > line is flexible enough that in the future we could have a --cloud-init > method=SMBIOS/... for users to explicitly choose what injection method > they want, but otherwise virt-install will choose a default, which may > be different depending on the arch. That sounds reasonable. > >> * Do you have an opinion of what behavior bare '--cloud-init' should give? > > > > My feeling is that a bare --cloud-init should inject the current user's > > SSH identity and/or authorized keys list. > > > > ie either we grab $HOME/.ssh/id_rsa.pub, or we grab all the keys from > > the SSH agent connection (ie ssh-add -l) > > > > Given that virt-intsall is run as root, there probably isn't a > > /root/.ssh/id_rsa.pub file for most people. Grabbing the ssh-agent > > authorized keys is probably best default approach. > > How to handle ssh keys kinda makes my head spin. I've shrunk the CC list > on this mail to just the folks that mentioned ssh in some way in their > replies > > If I'm thinking of the ideal interactive virt-install UI, it would be: > > $ virt-install --name MYVM --disk Fedora-Cloud-...qcow2 --cloud-init > INFO: SSH keys added to MYVM > INFO: Connecting to MYVM with: ssh fed...@192.168.122.xxx > ... > [fedora@localhost:~] $ > > > If all the pieces come together, that's pretty nice. But there's a lot > of missing pieces needed for that, and some unclear bits: > > * What is the cloud-init user name? Passing in an ssh key gives access > to the default cloud-init configured username, which is different > depending on the distro (might be fedora, ubuntu, etc). To properly > handle this, we either need libosinfo distro detection, or use > libguestfs to fish the default user out of /etc/cloud/cloud.cfg in the > VM. And root account is typically locked, and ssh access disabled, and > even if we could hack things to give root ssh access it seems like going > against the grain WRT cloud images. IIRC, you can pass an SSH key for root separately, but not sure how widely supported that is. Sticking to just providing 1 SSH key for the default unprivileged user likely suffices. > So best I can think of for an immediate solution for SSH is to print > something explanatory: > > INFO: you can ssh to the VM using the distro's default username. This is > usually one of: fedora, ubuntu, ... > > Or something along those lines. Maybe we can tie that into --os-variant > as well so the user can inform us what distro it is. Still it will take > some work. This is a bit of a tangent, but this makes me wonder if qcow2 format could benefit from having support for arbitrary user metadata fields People building cloud images could then tag their images with useful things including vendor, product name, product version, whether it supports cloud-init or ignition, what the default user account name is, suggested min ram, and so on. Of course people might say this is all stuff that's within scope of external metadata like OVF, but I've rarely, if ever, seen anyone say anything nice about OVF. Also there's a certain convenience in being able to directly pass around a bootable disk image, rather than a disk image + separate metadata file, or worse a ZIP file containing both which you have to unpack. Having some simple metadata in the qcow2 file would simplify life I think. If someone
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Thu, Nov 21, 2019 at 09:39:46PM -0500, Dusty Mabe wrote: > > > On 11/21/19 6:20 AM, Daniel P. Berrangé wrote: > > On Thu, Nov 21, 2019 at 11:07:24AM +, Richard W.M. Jones wrote: > >> On Thu, Nov 21, 2019 at 10:34:14AM +, Daniel P. Berrangé wrote: > >>> On Wed, Nov 20, 2019 at 08:18:01PM -0500, Dusty Mabe wrote: > >>>> Basically in Fedora CoreOS we need a generic user data mechanism that > >>>> works across > >>>> platforms (x86_64, aarch64, ppc64le, s390x) and doesn't have possible > >>>> race conditions. > >>>> Right now we're using `-fw_cfg` but it's not cross platform. We don't > >>>> have an answer > >>>> yet: https://github.com/coreos/ignition/issues/666#issuecomment-452835654 > >>> > >>> For platform portability you need to find some device that is common > >>> across all platforms, and either disk or network are the only two > >>> good options that exist today or for the forseeable future. If those > >>> aren't acceptable then all we have left are platform specific options. > >> > >> While it's not a "good option that exists today", AF_VSOCK would be a > >> good choice to settle on in the future. It's completely cross > >> platform, available for Windows, and doesn't interfere with existing > >> network or disk devices. > >> > >> Would needing virtio be a barrier? Our impl of AF_VSOCK runs over > >> virtio, but there are other transports. > > > > From a cloud-init POV, I don't see virtio as a barrier. Defining an > > AF_VSOCK data source for it should be quite straightforward really > > and they already have so many data sources, it seems reasonable > > that they'd accept one more. > > > > On the host side there's still the task of providing a metdata > > service to expose the data, which is outside scope of virt-install. > > > > Thanks for the fruitful conversation here regarding a cross platform data > source > that we could use. Is this worth us writing up into a request for an issue > tracker > somewhere where it could be discussed further? Possibly - depends if AF_VSOCK is viable or not for Ignition. Previously when we suggested use of a plain disk for Ignition, the issue raised was that the /dev/sdXXX device nodes take a non-deterministic amount of time to appear, and ignition doesn't know how long to wait for them, because the disks might not exist at all in the first place. Using AF_VSOCK is going to have exactly this same problem, so personally I can't see it can satisfy Ignition, where virtio-blk failed. The main benefit I see of AF_VSOCK over virtio-blk disk / cdrom, is that it is a live data channel so you are not restricted to providing data that is fixed at boot time, it can be live updated on the fly. The only mechanism that can avoid the problem of waiting for some device driver to initialize asynchonrously is to use a directly mapped memory region that is immediately exposed by the core kenrel functionality. This is what fw_cfg, SMBIOS, and/or kernel command line args all achieve, and the other options not based on mapped memory will all fail to achieve. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Thu, Nov 21, 2019 at 12:04:11PM +0100, Christian Ehrhardt wrote: > On Thu, Nov 21, 2019 at 11:52 AM Florian Weimer wrote: > > > > * Daniel P. Berrangé: > > > > >> This goes probably in a different direction of what has been implement > > >> so far, but would it actually harm to enable the network-based > > >> instance-data injection by default? The advantage would be that it also > > >> blocks these requests from leaking to untrusted parties, which could > > >> then serve bogus data to compromise the virtual machine. > > > > > > I don't understand what you mean by leaking data to untrusted parties > > > here in contetx of config drive ? I've considerd the config drive to > > > be more secure / less risky than network service. > > > > I'm assuming that cloud-init will try all sources in parallel, given > > that there's a delay for both the network coming about and hardware > > being detected. > > Hi, > there are many controls to that. By default it is most configurable, > but you can set it to your needs of e.g. only local data sources. > > As outlined by Daniel already this is pretty safe, but if still > concerned about it, you can control it [1]: > - image builders can disable things by a drop in file that controls > which sources are queried > - local users can control it via kernel-commandline (which most tools > provide an option to append things to) With pre-built disks images, virt-install can't directly control the kernel command line without using a tool like guestfish to get inside the image & modify grub config. Cloud-init can, however, look at SMBIOS to extract the information for the specific data source to use. Currently it is abusing the system-serial-number field for this purpose. I proposed a patch to make it use the SMBIOS OEM strings field instead https://bugs.launchpad.net/cloud-init/+bug/1753558 Either way though, virt-install can set the SMBIOS data in the guest to explicitly tell cloud-init to only use the configdrive ISO, and thus prevent it ever talking to the network metdata service. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Thu, Nov 21, 2019 at 12:06:49PM +0100, Florian Weimer wrote: > * Daniel P. Berrangé: > > > On Thu, Nov 21, 2019 at 11:52:26AM +0100, Florian Weimer wrote: > >> * Daniel P. Berrangé: > >> > >> >> This goes probably in a different direction of what has been implement > >> >> so far, but would it actually harm to enable the network-based > >> >> instance-data injection by default? The advantage would be that it also > >> >> blocks these requests from leaking to untrusted parties, which could > >> >> then serve bogus data to compromise the virtual machine. > >> > > >> > I don't understand what you mean by leaking data to untrusted parties > >> > here in contetx of config drive ? I've considerd the config drive to > >> > be more secure / less risky than network service. > >> > >> I'm assuming that cloud-init will try all sources in parallel, given > >> that there's a delay for both the network coming about and hardware > >> being detected. > > > > IIRC, the network sources all use link-local addresses, so by default > > you would need to have configured the 169.254.169.254 on one of the > > NICs on the host that the guest can reach. It connects to port 80 on > > this address. > > Too many IPv4 deployment treat 169.254.0.0/16 as global unicast > addresses and forward them via the default route. Only once they reach > the DFZ, these packets get dropped, but only if no one has announced a > route for it. Ah, I see what you mean now. > The instance-data DNS lookup is typically forwarded to the DNS root > servers. Local resolvers will only filter it if they are > DNSSEC-enabled. > > I have argued for a long time that separate cloud and local KVM images > are needed because the cloud images are dangerous in a non-cloud > environment, but so far without success. Libvirt has support for per-guest NIC network filters and ships with a "clean-traffic" filter that blocks ARP, IP & MAC spoofing. We could use this feature as a way to block access to the cloud-init metadata service IP address if desired. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Thu, Nov 21, 2019 at 11:07:24AM +, Richard W.M. Jones wrote: > On Thu, Nov 21, 2019 at 10:34:14AM +0000, Daniel P. Berrangé wrote: > > On Wed, Nov 20, 2019 at 08:18:01PM -0500, Dusty Mabe wrote: > > > Basically in Fedora CoreOS we need a generic user data mechanism that > > > works across > > > platforms (x86_64, aarch64, ppc64le, s390x) and doesn't have possible > > > race conditions. > > > Right now we're using `-fw_cfg` but it's not cross platform. We don't > > > have an answer > > > yet: https://github.com/coreos/ignition/issues/666#issuecomment-452835654 > > > > For platform portability you need to find some device that is common > > across all platforms, and either disk or network are the only two > > good options that exist today or for the forseeable future. If those > > aren't acceptable then all we have left are platform specific options. > > While it's not a "good option that exists today", AF_VSOCK would be a > good choice to settle on in the future. It's completely cross > platform, available for Windows, and doesn't interfere with existing > network or disk devices. > > Would needing virtio be a barrier? Our impl of AF_VSOCK runs over > virtio, but there are other transports. >From a cloud-init POV, I don't see virtio as a barrier. Defining an AF_VSOCK data source for it should be quite straightforward really and they already have so many data sources, it seems reasonable that they'd accept one more. On the host side there's still the task of providing a metdata service to expose the data, which is outside scope of virt-install. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Thu, Nov 21, 2019 at 11:52:26AM +0100, Florian Weimer wrote: > * Daniel P. Berrangé: > > >> This goes probably in a different direction of what has been implement > >> so far, but would it actually harm to enable the network-based > >> instance-data injection by default? The advantage would be that it also > >> blocks these requests from leaking to untrusted parties, which could > >> then serve bogus data to compromise the virtual machine. > > > > I don't understand what you mean by leaking data to untrusted parties > > here in contetx of config drive ? I've considerd the config drive to > > be more secure / less risky than network service. > > I'm assuming that cloud-init will try all sources in parallel, given > that there's a delay for both the network coming about and hardware > being detected. IIRC, the network sources all use link-local addresses, so by default you would need to have configured the 169.254.169.254 on one of the NICs on the host that the guest can reach. It connects to port 80 on this address. Thus to be able to serve malicious data by spoofing the network metadata service, the host would first need this IP address to be configured on a NIC, an second something needs to bind to port 80. Both of these steps require that you already have root on the host system, so I think the risk here is negligible. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Thu, Nov 21, 2019 at 08:40:12AM +0100, Florian Weimer wrote: > * Cole Robinson: > > > One more point: my main interaction with cloud-init has historically > > been by grabbing a Fedora/RHEL cloud image, passing it to > > virt-install/virt-manager, and watching the boot hang, because there's > > no data provider and cloud-init times out talking to the network, and > > then I can't log in. I expect many people have hit this issue before. > > I've always worked around this by using 'virt-customize' to disable > > cloud-init and reset the root password. That's about the extent of my > > usage here, which is broadly why the bare `--cloud-init` is the way it was. > > This is also my use case. 8-/ > > > I'm also thinking to the future, if one day virt-install can detect that > > it was passed a distro cloud-init image, perhaps we can invoke some > > default behavior that gives the user a better chance of this config > > being usable out of the box. I figure that will match whatever we choose > > for the bare '--cloud-init' behavior > > This goes probably in a different direction of what has been implement > so far, but would it actually harm to enable the network-based > instance-data injection by default? The advantage would be that it also > blocks these requests from leaking to untrusted parties, which could > then serve bogus data to compromise the virtual machine. I don't understand what you mean by leaking data to untrusted parties here in contetx of config drive ? I've considerd the config drive to be more secure / less risky than network service. Use of the ISO image should really minimize the risk of unexected data leakage, because the ISO image is restricted access on the host and is only granted to the specific QEMU process, with SELinux enforcement between each VM & other processes/users on the host. With a network based service there are many greater risks of data leakage I think. There is no SELinux separation between data for each VM. The metadata service holds all the data for all VMs. There's also the risk of the metdata service being accessed from processes on the host, or even on the WAN. If the network service is placed on a separate subnet, then you've got the mgmt problem of creating an extra NIC for the guest and extra device on the host to attach it to, and managing firewall rules to correctly isolate the metadta service from unintended access. Many OSP deployments refuse to enable the metadata service because they consider it exposes more attack vectors than configdrive does. Having said all that metadata service can be useful from a managability POV because it is a live data channel, and thus enables you to change the data being served while the VM is running. This can be useful for the userdata blob. Config drive is fixed at time the VM boots. Network metadata service though is out of scope for virt-install. If someone ones to deploy one they can do so already today without needing any more features in virt-install. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Wed, Nov 20, 2019 at 08:18:01PM -0500, Dusty Mabe wrote: > > > On 11/20/19 5:49 PM, Cole Robinson wrote: > > Hi all. The purpose of this mail is to get some feedback on pending > > cloud-init support in virt-install. If you're on the CC list here, I > > either pulled your email from a cloud-init discussion on the the > > virt-tools-list mailing list, or from the CC list of this virt-install bug: > > > > RFE: Provide cloud-init integration for VMs > > https://bugzilla.redhat.com/show_bug.cgi?id=981693 > > > > For GSOC 2019 Athina Plaskasoviti completed some cloud-init integration > > work in virt-install. You can read her wrap up here: > > https://athinapl.home.blog/2019/08/25/gsoc-2019-cloud-init-configuration-for-virt-manager-virt-install/ > > First off, thanks Cole and Athina for working on this. This is something I > have > wanted for a long time! In my dayjob I've moved away from cloud-init a bit > because > our new platforms are running Ignition: https://github.com/coreos/ignition > > From the Ignition side I've added Andrew Jeddeloh and Benjamin Gilbert to CC. > > > > > Right now the code is sitting in a virt-manager.git branch, not yet > > pushed to master: > > https://github.com/virt-manager/virt-manager/tree/cloudinit > > > > I'll summarize the current behavior, and then ask some questions. > > > > > > The branch adds a new 'virt-install --cloud-init' argument with several > > sub options. When specified, virt-install generates an empty meta-data > > file, a user-data file with the requested changes, stuffs them both into > > a cidata iso, which is used for the first VM boot and then deleted. > > Is there any chance we could go a different route than an ISO image? The > developers > of ignition have experienced problems with relying on a device here (i.e. is > there no device coming or has the device just not shown up yet?). Some > discussion over > in: https://github.com/coreos/ignition/issues/656 > > Basically in Fedora CoreOS we need a generic user data mechanism that works > across > platforms (x86_64, aarch64, ppc64le, s390x) and doesn't have possible race > conditions. > Right now we're using `-fw_cfg` but it's not cross platform. We don't have an > answer > yet: https://github.com/coreos/ignition/issues/666#issuecomment-452835654 For platform portability you need to find some device that is common across all platforms, and either disk or network are the only two good options that exist today or for the forseeable future. If those aren't acceptable then all we have left are platform specific options. I think relying on platform specific options *is* acceptable, provided we can identify a viable option for each platform. We have SMBIOS OEM strings as recommended solution for x86 and aarch64. We are yet to identify any good options for ppc or s390 though. Perhaps we can define a mapping for SMBIOS into these arches, as SMBIOS is ultimately just a spec for a set of tables in a region of memory. It possible, probably even likely, that we will have to choose something entirely different though for these arches. > > This behavior is only triggered when --cloud-init is specified in some > > form, there's no automagic invocation of this support. > > > > The command sub options are: > > > > $ ./virt-install --cloud-init=help > > --cloud-init options: > > disable > > root-password-file > > root-password-generate > > ssh-key > > user-data > > > > Their behavior: > > > > * disable=yes: boolean option to disable cloud-init in the VM for > > subsequent boots. Adds this block to user-data: > > runcmd: > > - [ sudo, touch, /etc/cloud/cloud-init.disabled ] > > > > * root-password-file=/MY/PATH: set the desired root password from the > > content of /MY/PATH on the host > > > > * root-password-generate=yes: boolean option, generate a random root > > password, set it in user-data, print it to the host text console and > > pause for a bit for the user to see and copy it. sorta inspired by > > virt-builder > > > > * ssh-key=/MY/KEY.pub: inject the ssh key from /MY/KEY.pub on the host > > into the cloud-init user-data > > > > * user-data=/PATH/TO/user-data: ignore all other options and copy this > > file to the .iso as user-data > > > > When bare '--cloud-init' is specified, we default to generating a random > > root password and disabling cloud-init for subsequent boots: > > --cloud-init root-password-generate=yes,disable=yes > > > > We've explicitly rejected something like root-password=MY-PASSWORD > > because of the security implications of encouraging a password to end up > > in command line history. We've already had a CVE for something similar > > in virt-install. > > > > Also, I don't want virt-install to be in the business of specifying > > every cloud-init option under the sun, there's gotta be a better tool > > for that already. So I'd like to keep --cloud-init suboptions > > specifically targeted to expected virt use cases, and anything else can > >
Re: [virt-tools-list] virt-install and cloud-init, feedback wanted
On Wed, Nov 20, 2019 at 05:49:45PM -0500, Cole Robinson wrote: > Hi all. The purpose of this mail is to get some feedback on pending > cloud-init support in virt-install. If you're on the CC list here, I > either pulled your email from a cloud-init discussion on the the > virt-tools-list mailing list, or from the CC list of this virt-install bug: > > RFE: Provide cloud-init integration for VMs > https://bugzilla.redhat.com/show_bug.cgi?id=981693 > > For GSOC 2019 Athina Plaskasoviti completed some cloud-init integration > work in virt-install. You can read her wrap up here: > https://athinapl.home.blog/2019/08/25/gsoc-2019-cloud-init-configuration-for-virt-manager-virt-install/ > > Right now the code is sitting in a virt-manager.git branch, not yet > pushed to master: > https://github.com/virt-manager/virt-manager/tree/cloudinit > > I'll summarize the current behavior, and then ask some questions. > > > The branch adds a new 'virt-install --cloud-init' argument with several > sub options. When specified, virt-install generates an empty meta-data > file, a user-data file with the requested changes, stuffs them both into > a cidata iso, which is used for the first VM boot and then deleted. > > This behavior is only triggered when --cloud-init is specified in some > form, there's no automagic invocation of this support. > > The command sub options are: > > $ ./virt-install --cloud-init=help > --cloud-init options: > disable > root-password-file > root-password-generate > ssh-key > user-data > > Their behavior: > > * disable=yes: boolean option to disable cloud-init in the VM for > subsequent boots. Adds this block to user-data: > runcmd: > - [ sudo, touch, /etc/cloud/cloud-init.disabled ] > > * root-password-file=/MY/PATH: set the desired root password from the > content of /MY/PATH on the host > > * root-password-generate=yes: boolean option, generate a random root > password, set it in user-data, print it to the host text console and > pause for a bit for the user to see and copy it. sorta inspired by > virt-builder > > * ssh-key=/MY/KEY.pub: inject the ssh key from /MY/KEY.pub on the host > into the cloud-init user-data > > * user-data=/PATH/TO/user-data: ignore all other options and copy this > file to the .iso as user-data > > When bare '--cloud-init' is specified, we default to generating a random > root password and disabling cloud-init for subsequent boots: > --cloud-init root-password-generate=yes,disable=yes > > We've explicitly rejected something like root-password=MY-PASSWORD > because of the security implications of encouraging a password to end up > in command line history. We've already had a CVE for something similar > in virt-install. > > Also, I don't want virt-install to be in the business of specifying > every cloud-init option under the sun, there's gotta be a better tool > for that already. So I'd like to keep --cloud-init suboptions > specifically targeted to expected virt use cases, and anything else can > be served with custom user-data= > > One more point: my main interaction with cloud-init has historically > been by grabbing a Fedora/RHEL cloud image, passing it to > virt-install/virt-manager, and watching the boot hang, because there's > no data provider and cloud-init times out talking to the network, and > then I can't log in. I expect many people have hit this issue before. > I've always worked around this by using 'virt-customize' to disable > cloud-init and reset the root password. That's about the extent of my > usage here, which is broadly why the bare `--cloud-init` is the way it was. > > I'm also thinking to the future, if one day virt-install can detect that > it was passed a distro cloud-init image, perhaps we can invoke some > default behavior that gives the user a better chance of this config > being usable out of the box. I figure that will match whatever we choose > for the bare '--cloud-init' behavior One option is to take the lazy approach and always enable cloud init if we are given a pre-built disk image. * If the disk image doesn't support cloud init, then its fairly harmless to provide cloud-init data. * If the disk image does support cloud init but has already been booted, and the user has marked cloud init as disable, then it is again harmless to provide cloud-init data * If the disk image does support cloud init but has already been booted, and the user left cloud init enabled so that it runs on every boot, then passing cloud init data is probably good thing. * If the disk image does support cloud init and has never been booted, then obviously providing cloud init data is good > * What are the usecases you see for virt-install cloud-init support? To me the critical use case is simply providing a way for the user to login to the guest. This is covered by password and/or SSH key inject. Once they have that working, anything else is just nice to have. > * Does the above meet your expectations?
Re: [virt-tools-list] [PATCH 0/3] virt-viewer: add SIGINT handler
On Tue, Nov 12, 2019 at 12:29:09PM +0100, Francesco Giudici wrote: > When remote-viewer or virt-viewer are terminated by a signal, they quit > without explicitly releasing resources. A bug[1] has been filed against > virt-viewer for not releasing redirected usb devices when it's terminated > by CTRL-C. Clearly this could be solved by adding a signal handler doing > proper shutdown of the application. All resources used by a process are automatically released by the kernel when the process exits. IOW, doing a "graceful exit" to manually release USB devices should not be required, not least because we want everything to behave well even in the face of an abnormal application crash where we have no way to manually releasing devices. Can you explain what is preventing the USN device release from the kernel POV ? 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH 3/3] remote-viewer: allow to build with Microsoft Visual Studio
On Tue, Nov 12, 2019 at 12:29:12PM +0100, Francesco Giudici wrote: > In order to meaningfully use the g_io_channel_win32_new_fd() function > the C runtime used by GLib is required, which is msvcrt.dll. In current > Microsoft compilers it is near impossible to convince it to build code > that would use msvcrt.dll. The last Microsoft compiler version that > supported using msvcrt.dll as the C runtime was version 6. The GNU > compiler and toolchain for Windows, also known as Mingw, fully supports > msvcrt.dll. > For this reason, drop the SIGINT handler when building with Microsoft > Visual Studio. IMHO we should explicitly not support the microsoft compilers at all. Just mandate use of GCC or CLang, as they are available for every OS platform that matters and support a much better feature set and would allow us to simplify code by making use of C extensions such as "g_autoptr". 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [RFC virt-viewer] virt-viewer-app: force quit if quit is requested twice
On Thu, Oct 31, 2019 at 12:19:17PM +0100, Kevin Pouget wrote: > The investigation of BZ#1713548 shows that the viewer cannot be closed > if Spice-GTK does not send the DISCONNECTED signal [1]. > > This patch allows a 'force-close' if the user requests twice to close > the app: > > 1. The first time, > `virt_viewer_session_close(VIRT_VIEWER_SESSION(priv->session))` is called, > which should trigger the DISCONNECTED signal and close the app. If this fails, > 2. The second time, the application is closed, no matter the internal state. > > See [2] for the patch that introduced the two-step disconnection. > > 1: > https://gitlab.freedesktop.org/spice/spice-gtk/blob/0c52ce8937c849d8ae32ade1f22ce3a48c56c732/src/spice-session.c#L2322 > 2: https://pagure.io/virt-viewer/c/8ec03e50 > --- > src/virt-viewer-app.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) If there is a problem causing spice-gtk to not reliably send the DISCONNECTED signal, we should be fixing spice, rather than hacking around the problem in apps. If fixing spice-gtk is not possible, then we should not wait for DISCONNECTED at all, just quit immediately. Exiting the app will close all the connections anyway and the spice server needs to support cleanup in this way regardless. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/2] unattended: Don't log user & admin passwords
On Wed, Jul 03, 2019 at 02:42:30PM +0200, Fabiano Fidêncio wrote:
> On Wed, Jul 3, 2019 at 1:54 PM Pavel Hrdina wrote:
> >
> > On Tue, Jul 02, 2019 at 09:21:45PM +0200, Fabiano Fidêncio wrote:
> > > Logging user & admin passwords in the command-line is a security issue,
> > > let's avoid doing so by:
> > > - Not printing the values set by the user when setting up the
> > > install-script config file;
> > > - Removing the values used in the install-scripts, when printing their
> > > content;
> > >
> > > 'CVE-2019-10183' has been assigned to the virt-install --unattended
> > > admin-password=xxx disclosure issue.
> > >
> > > Signed-off-by: Fabiano Fidêncio
> > > ---
> > > virtinst/install/unattended.py | 10 --
> > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/virtinst/install/unattended.py
> > > b/virtinst/install/unattended.py
> > > index 4f311296..04758563 100644
> > > --- a/virtinst/install/unattended.py
> > > +++ b/virtinst/install/unattended.py
> > > @@ -97,8 +97,6 @@ def _make_installconfig(script, osobj, unattended_data,
> > > arch, hostname, url):
> > > log.debug("InstallScriptConfig created with the following params:")
> > > log.debug("username: %s", config.get_user_login())
> > > log.debug("realname: %s", config.get_user_realname())
> > > -log.debug("user password: %s", config.get_user_password())
> > > -log.debug("admin password: %s", config.get_admin_password())
> > > log.debug("target disk: %s", config.get_target_disk())
> > > log.debug("hardware arch: %s", config.get_hardware_arch())
> > > log.debug("hostname: %s", config.get_hostname())
> > > @@ -195,6 +193,14 @@ class OSInstallScript:
> > > content = self.generate()
> > > open(scriptpath, "w").write(content)
> > >
> > > +user_password = self._config.get_user_password()
> > > +if user_password:
> > > +content = content.replace(user_password, "[SCRUBBED]")
> > > +
> > > +admin_password = self._config.get_admin_password()
> > > +if admin_password:
> > > +content = content.replace(admin_password, "[SCRUBBED]")
> >
> > There is a small issue with this approach, if you for testing purposes
> > or for any other reason use password that matches anything else in the
> > script file (kickstart for example) it will be replaced as well:
> >
> > ""
> > %post --erroronfail
> >
> > useradd -G wheel phrdina # Add user
> > if [SCRUBBED] -z ''; then
> > passwd -d phrdina # Make user account passwordless
> > else
> > echo '' |passwd --stdin phrdina
> > fi
> >
> > if [SCRUBBED] -z '[SCRUBBED]'; then
> > passwd -d root # Make root account passwordless
> > else
> > echo '[SCRUBBED]' |passwd --stdin root
> > fi
> > "
> >
> > Here I used as a password 'test' and you can see the test command was
> > replaced as well. Probably not a big deal is it modifies only the log
> > output, not the actual file but I thought that I should at least point
> > to this corner case issue. Do we care about it or not?
>
> I thought about that and I sincerely don't care much about that.
> Otherwise we'll have to learn exactly what to match in all the install
> scripts supported (as in, kickstarts, autoyast, jeos, ...).
You can avoid this kind of matching at all but just using a different
Libosinfo.InstallConfig. eg just copy the master config and set the
password params to a sanitized value & generate a new script.
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 :|
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH v3] Introduction of cloud-init configuration in virt-install
On Fri, Jun 28, 2019 at 07:05:18PM +0300, Athina Plaskasoviti wrote:
> Usage:
> --cloud-init
>
> Signed-off-by: Athina Plaskasoviti
> ---
> virt-install| 5 +++
> virtinst/cli.py | 25 +
> virtinst/install/cloudinit.py | 57 +
> virtinst/install/installer.py | 17 +
> virtinst/install/installerinject.py | 20 +-
> 5 files changed, 115 insertions(+), 9 deletions(-)
> create mode 100644 virtinst/install/cloudinit.py
>
> diff --git a/virt-install b/virt-install
> index ee2b9006..1ccf7a31 100755
> --- a/virt-install
> +++ b/virt-install
> @@ -444,6 +444,9 @@ def build_installer(options, guest, installdata):
> installer.set_initrd_injections(options.initrd_inject)
> if options.autostart:
> installer.autostart = True
> +if options.cloud_init:
> +cloudinit_data = cli.parse_cloud_init(options.cloud_init)
> +installer.set_cloudinit_data(cloudinit_data)
>
> return installer
>
> @@ -821,6 +824,8 @@ def parse_args():
> help=_("Perform an unattended installation"))
> insg.add_argument("--install",
> help=_("Specify fine grained install options"))
> +insg.add_argument("--cloud-init", nargs="?", const=1,
> +help=_("Perform a cloud image installation, configuring
> cloud-init"))
>
> # Takes a URL and just prints to stdout the detected distro name
> insg.add_argument("--test-media-detection", help=argparse.SUPPRESS)
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index 9a1fe2f6..9a6badcd 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -28,6 +28,7 @@ from .nodedev import NodeDevice
> from .osdict import OSDB
> from .storage import StoragePool, StorageVolume
> from .install.unattended import UnattendedData
> +from .install.cloudinit import CloudInitData
>
>
> ##
> @@ -1602,6 +1603,30 @@ def parse_install(optstr):
> return installdata
>
>
> +
> +# --cloud-init parsing #
> +
> +
> +class ParserCloudInit(VirtCLIParser):
> +cli_arg_name = "cloud-init"
> +
> +@classmethod
> +def _init_class(cls, **kwargs):
> +VirtCLIParser._init_class(**kwargs)
> +cls.add_arg("root-password", "root_password")
> +
> +
> +def parse_cloud_init(optstr):
> +ret = CloudInitData()
> +if optstr == 1:
> +# This means bare --cloud-init, so there's nothing to parse
> +return ret
> +
> +parser = ParserCloudInit(optstr)
> +parser.parse(ret)
> +return ret
> +
> +
> ##
> # --location parsing #
> ##
> diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
> new file mode 100644
> index ..41667f4b
> --- /dev/null
> +++ b/virtinst/install/cloudinit.py
> @@ -0,0 +1,57 @@
> +import tempfile
> +import random
> +import string
> +import time
> +from ..logger import log
> +
> +
> +class CloudInitData():
> +root_password = None
> +
> +
> +def create_metadata(scratchdir, hostname=None):
> +if hostname:
> +instance = hostname
> +else:
> +hostname = instance = "localhost"
> +content = 'instance-id: %s\n' % instance
> +content += 'hostname: %s\n' % hostname
> +log.debug("Generated cloud-init metadata:\n%s", content)
> +
> +fileobj = tempfile.NamedTemporaryFile(
> +prefix="virtinst-", suffix="-metadata",
> +dir=scratchdir, delete=False)
> +filename = fileobj.name
> +
> +with open(filename, "w") as f:
> +f.write(content)
> +return filename
> +
> +
> +def create_userdata(scratchdir, cloudinit_data, username=None,
> password=None):
> +if not password:
> +password = ""
> +for dummy in range(16):
> +password += random.choice(string.ascii_letters + string.digits)
> +content = "#cloud-config\n"
> +if username:
> +content += "name: %s\n" % username
> +if cloudinit_data.root_password == "generate":
> +pass
> +else:
> +content += "password: %s\n" % password
> +log.debug("Generated password for first boot: \n%s", password)
> +time.sleep(20)
> +content += "runcmd:\n"
> +content += "- [ sudo, touch, /etc/cloud/cloud-init.disabled ]\n"
> +log.debug("Generated cloud-init userdata:\n%s", content)
This message is going to leak the root password too. It is nice to
log the cloud init data though, so we need a fix to scrub the actual
root password from it before logging.
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 :|
___
virt-tools-list mailing list
virt-tools-list@redhat.com
Re: [virt-tools-list] [virt-manager PATCH v3] Introduction of cloud-init configuration in virt-install
On Fri, Jun 28, 2019 at 07:05:18PM +0300, Athina Plaskasoviti wrote:
> Usage:
> --cloud-init
>
> Signed-off-by: Athina Plaskasoviti
> ---
> virt-install| 5 +++
> virtinst/cli.py | 25 +
> virtinst/install/cloudinit.py | 57 +
> virtinst/install/installer.py | 17 +
> virtinst/install/installerinject.py | 20 +-
> 5 files changed, 115 insertions(+), 9 deletions(-)
> create mode 100644 virtinst/install/cloudinit.py
> +def create_userdata(scratchdir, cloudinit_data, username=None,
> password=None):
> +if not password:
> +password = ""
> +for dummy in range(16):
> +password += random.choice(string.ascii_letters + string.digits)
> +content = "#cloud-config\n"
> +if username:
> +content += "name: %s\n" % username
> +if cloudinit_data.root_password == "generate":
> +pass
> +else:
> +content += "password: %s\n" % password
> +log.debug("Generated password for first boot: \n%s", password)
> +time.sleep(20)
Sleeping to let the user see it is nice, but we should allow the user
to press enter to continue the operation instead of forcing them to
wait the full 20 seconds.
Also, never put passwds into log messages - log files are frequently
uploaded to public bug trackers, so having passwds in there will
violate users' privacy / security.
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 :|
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH][v2][RFC] Introduction of cloud-init configuration in virt-install
On Mon, Jun 24, 2019 at 01:53:38PM -0400, Cole Robinson wrote:
> On 6/24/19 12:40 PM, Daniel P. Berrangé wrote:
> > On Mon, Jun 24, 2019 at 12:12:16PM -0400, Cole Robinson wrote:
> >> On 6/21/19 1:18 PM, Athina Plaskasoviti wrote:
> >>> I am sending a v2 of the patch for cloud init basic configuration during
> >>> cloud image installation, applying previous suggestions.
> >>>
> >>> Fixed:
> >>>
> >>> -the way the installation is triggered (no_install is no longer required):
> >>> --install is_cloud=yes ... --import
> >>>
> >>> -added tmpfile for the files that are being generated in the installation
> >>> process
> >>> (meta-data, user-data, config.iso)
> >>>
> >>> -changed condition in installerinject.py from label='cloud' to
> >>> cloudinit=True
> >>>
> >>
> >>
> >> I did some digging into cloud-init behavior to try and get a better idea
> >> of what the minimal viable config is, and get ideas for future work.
> >>
> >>
> >> cloud-init behavior
> >> ===
> >>
> >> The patch generates an .iso with volume label 'cidata' which is the
> >> magic to make cloud-init search it for config data. cloud-init must find
> >> a 'meta-data' and 'user-data' file in order to consider the iso a valid
> >> data source. There isn't any requirements on the contents though. They
> >> can both basically be empty AFAICT.
> >>
> >> This patch is setting meta-data 'instance-id' and 'hostname' fields but
> >> neither are required: instance-id seems to only be needed on real cloud
> >> providers (and cloud-init fills in a 'nocloud' value for us by default).
> >> hostname will predictably set the VM hostname but that's not something
> >> we need to set by default. So in fact meta-data is fine to leave empty
> >> and I think that should be our default behavior.
> >>
> >> Every distro cloud image has some default config in
> >> /etc/cloud/cloud.cfg. This is generated from cloud-init.git
> >> tools/render-cloudcfg which generates cloud.cfg from
> >> config/cloud.cfg.tmpl. Basically the only input to the template is the
> >> distro name. For example fedora generates in in the RPM spec with simply:
> >>
> >> python3 tools/render-cloudcfg --variant fedora >
> >> $RPM_BUILD_ROOT/%{_sysconfdir}/cloud/cloud.cfg
> >>
> >> cloud images seem to have this default behavior:
> >> - ssh is enabled and running
> >> - root account is locked
> >> - cloud.cfg will explicitly disallow ssh to the root account, via
> >> disable_root. Attempts to ssh to root will print an error message
> >> pointing to the default account
> >> - cloud.cfg will create a default account, locked, with full sudo access.
> >>
> >> The name of the default account depends on the value passed to the
> >> tools/render-cloudcfg script. For Fedora it's 'fedora', Ubuntu is
> >> 'ubuntu'. The openstack page here documents the common ones:
> >> https://docs.openstack.org/image-guide/obtain-images.html
> >>
> >> An extra note: I was wrong when I said in other threads that fedora
> >> images have a default password, they just have a default account name
> >> which is locked by default.
> >
> > That good, so I was in fact right in remembering that.
> >
>
> Yeah, sorry for the confusion.
>
> >> virt-install cli
> >>
> >>
> >> I think the virt-install entry point for requesting cloud-init stuff
> >> should actually be its own top level option, like --unattended, because
> >> we may want to add suboptions and I don't want to mix those in with
> >> --install. So I'm thinking:
> >>
> >> # does whatever we choose for default behavior, like --install is_cloud
> >> virt-install --cloud-init ...
> >>
> >> # User provides their own user-data file. Maybe for meta-data too?
> >> virt-install --cloud-init user-data=/path/to/user-data
> >>
> >> # Specify password for the default user. Yeah I know this isn't
> >> # a good pattern to put a password on the cli but we could combine it
> >> # with immediate password expiration so the user has to change it
> >> # on first log in
> >> virt-install --cloud-init password=FOOBAR
> >>
> >> # Something like this to inject an ssh key? But honestly I'm not
> >> # convinced we should go
Re: [virt-tools-list] [virt-manager PATCH][v2][RFC] Introduction of cloud-init configuration in virt-install
On Mon, Jun 24, 2019 at 12:12:16PM -0400, Cole Robinson wrote:
> On 6/21/19 1:18 PM, Athina Plaskasoviti wrote:
> > I am sending a v2 of the patch for cloud init basic configuration during
> > cloud image installation, applying previous suggestions.
> >
> > Fixed:
> >
> > -the way the installation is triggered (no_install is no longer required):
> > --install is_cloud=yes ... --import
> >
> > -added tmpfile for the files that are being generated in the installation
> > process
> > (meta-data, user-data, config.iso)
> >
> > -changed condition in installerinject.py from label='cloud' to
> > cloudinit=True
> >
>
>
> I did some digging into cloud-init behavior to try and get a better idea
> of what the minimal viable config is, and get ideas for future work.
>
>
> cloud-init behavior
> ===
>
> The patch generates an .iso with volume label 'cidata' which is the
> magic to make cloud-init search it for config data. cloud-init must find
> a 'meta-data' and 'user-data' file in order to consider the iso a valid
> data source. There isn't any requirements on the contents though. They
> can both basically be empty AFAICT.
>
> This patch is setting meta-data 'instance-id' and 'hostname' fields but
> neither are required: instance-id seems to only be needed on real cloud
> providers (and cloud-init fills in a 'nocloud' value for us by default).
> hostname will predictably set the VM hostname but that's not something
> we need to set by default. So in fact meta-data is fine to leave empty
> and I think that should be our default behavior.
>
> Every distro cloud image has some default config in
> /etc/cloud/cloud.cfg. This is generated from cloud-init.git
> tools/render-cloudcfg which generates cloud.cfg from
> config/cloud.cfg.tmpl. Basically the only input to the template is the
> distro name. For example fedora generates in in the RPM spec with simply:
>
> python3 tools/render-cloudcfg --variant fedora >
> $RPM_BUILD_ROOT/%{_sysconfdir}/cloud/cloud.cfg
>
> cloud images seem to have this default behavior:
> - ssh is enabled and running
> - root account is locked
> - cloud.cfg will explicitly disallow ssh to the root account, via
> disable_root. Attempts to ssh to root will print an error message
> pointing to the default account
> - cloud.cfg will create a default account, locked, with full sudo access.
>
> The name of the default account depends on the value passed to the
> tools/render-cloudcfg script. For Fedora it's 'fedora', Ubuntu is
> 'ubuntu'. The openstack page here documents the common ones:
> https://docs.openstack.org/image-guide/obtain-images.html
>
> An extra note: I was wrong when I said in other threads that fedora
> images have a default password, they just have a default account name
> which is locked by default.
That good, so I was in fact right in remembering that.
> virt-install cli
>
>
> I think the virt-install entry point for requesting cloud-init stuff
> should actually be its own top level option, like --unattended, because
> we may want to add suboptions and I don't want to mix those in with
> --install. So I'm thinking:
>
> # does whatever we choose for default behavior, like --install is_cloud
> virt-install --cloud-init ...
>
> # User provides their own user-data file. Maybe for meta-data too?
> virt-install --cloud-init user-data=/path/to/user-data
>
> # Specify password for the default user. Yeah I know this isn't
> # a good pattern to put a password on the cli but we could combine it
> # with immediate password expiration so the user has to change it
> # on first log in
> virt-install --cloud-init password=FOOBAR
>
> # Something like this to inject an ssh key? But honestly I'm not
> # convinced we should go down that route and instead just make users
> # manually set this themselves like they would for any other VM
> virt-install --cloud-init sshkey=/path/to/key
I think ssh keys is the preferred way to setup cloud images in many
cases as no one sane should ever be using passwords for ssh auth these
days given the continual brute force attacks they face.
Saying users must set a password and then manually copy their ssh key
in afterwards is crazy when cloud init supports getting the ssh key
in straight away without needing passwords.
I would not be surprised to even find cloud images which have
explicitly disabled SSH passwords entirely leaving SSH keys as the
only option. Some organizations will have a complete ban on use of
passwords for login purposes too.
> --cloud-init default behavior
> =
>
> This is the bit I'm not really sure about. We need to manipulate some
> password in some way for the user to be able to log in.
>
> For starters I think no matter what option we choose we should set the
> password to expire on first log in, thus forcing the user to choose a
> password of their own.
>
> The safest: generate a long random password for the default user. Print
> it to the
Re: [virt-tools-list] RFC: virt-manager UI philosophy draft
On Wed, Jun 19, 2019 at 06:34:38PM -0400, Cole Robinson wrote: > Hi all, > > I've drafted a wiki page about virt-manager UI philosophy, for lack of a > better term, suggestions welcome. The intention here is to provide some > guidance about what types of things we want to expose in the UI, both to > devs, potential contributors, and users requesting RFEs. > > https://github.com/virt-manager/virt-manager/wiki/WIP-UI-philosophy > > I don't think there's much new here, it's just formalized. There's a > section at the end of the wiki page for previously rejected features as > well. If people are cool with this document I'll use it to close a bunch > of upstream bugs as well which will grow that list. > > Another part of my plan for this is to apply the standards to the > current UI which would result in feature removals. I'll put my thoughts > in a follow up mail. > > > ## virt-manager UI philosophy > > virt-manager is a UI toolbox-style frontend for libvirt. It provides UI > access to common virt management tasks and operations. virt-manager aims > to provide a simple UI, but not too simple that it excludes valid > usecases. virt-manager prioritizes stability over features. > > * _Basic virt users_ should be able to meet their needs with virt-manager. > * _Intermediate virt users_ should generally find virt-manager > sufficiently flexible for their needs. > * _Advanced virt users_ will not find explicit UI support for their > advanced use cases, but virt-manager should still function correctly in > the face of their manually configured advanced virt usage. virt-manager > should not get in their way. > > See the user definitions at the end of this doc for more details on those. > > Here are some things that virt-manager explicitly is not aiming to > reimplement: > * gnome-boxes: a heavily desktop integrated VM manager with an emphasis > on UI design and simplifying virt management. They prioritize a seamless > designed experience over flexibility, our goals are different. > * virt-viewer/remote-viewer, vncviewer: our graphical VM window should > 'just work' for most needs but any advanced console configuration is > left up to these other better suited tools. > * VirtualBox, VMWare Workstation: It's a nice idea to aim to be the > equivalent of those apps for the QEMU+KVM+Libvirt stack. But to get > there would require a level of resource investment that is unlikely to > ever happen. > * oVirt, Openstack: virt-manager does not aim to support management of > many hosts with many VMs. virt-manager won't reject this case and we try > within reason to keep it working. But the UI is not designed for it and > we will not change the UI to facilitate these style of usecases. > > > ## How do we evaluate UI changes > > When is it worth it to expose something in the UI? Here's some criteria > we will use: > * How many users do we expect will use it: This is handwavy of course > but some things come up in email/IRC discussion regularly, and some are > mentioned once in 5 years. > * How critical is it for users who need/want it: if it's an absolute > blocker just to get a working config for some people, that can influence > the discussion > * How self explanatory is the feature: 'Enable 3D acceleration' is > fairly self explanatory. Disk io native vs threads, not so much. > * How dangerous or difficult to use is the feature: If it works in only > specific cases or renders the VM unbootable for certain scenarios, this > matters. > * How much work is it to maintain, test > * How much work is it to implement: If something requires significant > app specific logic on top of libvirt, libosinfo, or spice-gtk that would > also be useful to other virt apps, it is suspect. We should be aiming to > share common functionality > > > ## User definitions > > ### Basic virt user > They know little or nothing about libvirt, qemu, and kvm, but they > understand the high level concept of a virtual machine. They have a > Windows or Linux distro ISO and they want to create a VM and interact > with it graphically. They should be able to figure out how to do that by > running virt-manager and following the UI. The defaults we provide for > new VMs should be sufficient for their needs. > > After the VM is installed, the UI should facilitate intuitive UI tasks like: > > * lifecycle operations: start/stop/pause the VM; save, snapshot the VM; > delete, clone the VM > * rename the VM; change the VM title or description > * eject/insert CDROM media; change VM boot order > * increase VM memory > * attach a host USB device to the VM; possibly add an additional disk to > the VM > * graphical operations like send a keycombo, screenshot I would note that this is essentially the use case / scenario that GNOME Boxes aims to fill. Boxes will inevitably do a better job here since its simplified view will enable it to be more friendly than virt-manager will be able to do. IMHO virt-manager should position itself as the natural choice of
Re: [virt-tools-list] RFC: virt-manager feature removals
On Fri, Jun 21, 2019 at 07:14:56PM -0400, Cole Robinson wrote: > On 6/19/19 6:34 PM, Cole Robinson wrote: > > Hi all, > > > > I've drafted a wiki page about virt-manager UI philosophy, for lack of a > > better term, suggestions welcome. The intention here is to provide some > > guidance about what types of things we want to expose in the UI, both to > > devs, potential contributors, and users requesting RFEs. > > > > https://github.com/virt-manager/virt-manager/wiki/WIP-UI-philosophy > > Following on from the UI philosophy document, here's a list of features > in current virt-manager/virt-install features that I think are > candidates for removal. > * the migration wizard: migration is IMO an advanced use case. It > requires host config outside of virt-manager to have a chance of > working. the only place migration is really critical for users is > serious multihost production scenarios where virt-manager isn't really > appropriate per the UI philosophy. And besides some UI friendliness > virt-manager doesn't really add much over virsh IMO. Nearly every bug I > can recall about its usage was from one person who happens to be a > co-worker developing qemu migration, and redhat QE, which means it's > either perfect or no one uses it :) I think it should go I'm not really convinced by this, as I think it dumbs down virt-manager too much. Telling GUI users to use virsh instead is not a very compelling story IMHO. virt-manager has multi-host as a feature, and so I think it is reasonable to expect it to support migration. It isn't only large openstack scale deployments that can find it useful. > * console scaling options (always, never, fullscreen): we've had this in > the UI forever, but I don't think it has much usage. virt-viewer doesn't > even expose it how we do, instead it just has a zoom option which I > don't think we need to implement either. just today there was a bug > reported saying that scale->always is fairly obviously broken and has > been for a few releases, which makes me think no one is using it. > dropping this could be part of a larger think of how console sizing > works, if we should default to spice auto-resize, or if there's some > more modern config we should be taking care of. > https://bugzilla.redhat.com/show_bug.cgi?id=1722088 The right answer is different for VNC vs SPICE. For VNC you just want to resize the main window to fit, if possible, otherwise scale it down. No compelling reason to want to be scaling up. For SPICE though if the guest agent is present then you want to guest display to auto-resize to match whatever the host window allows to display. That way you never need scaling down. > * graphics keymap stuff. in virtinst/hostkeymap.py we have some logic > to parse host keymap from various locations like /etc/vconsole.conf and > try to map it to a qemu keymap name. You can trigger this via the UI in > the keymap dropdown 'Copy local keymap' and via virt-install --graphics > keymap=local, though the latter isn't even documented. This all was > the default behavior we used and essentially needed years ago, but > qemu and spice-gtk/gtk-vnc have for a decade been able to route > around the need by sending raw scancodes back and forth. This code > could in theory still be valuable for someone to set the keymap to > work with a vncviewer for example which doesn't have the scancode > support IIRC, but I think at that point we can just ask them to specify > the keymap themselves. So I propose killing hostkeymap entirely, making > keymap=local print a cli warning, and drop the keymap dropdown from the > UI. Users can set it in the XML editor if they need it Yes, there's no compelling reason to set the keymap - even non-gtk-vnc based clients like tigervnc support the scancode mode now. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH][v2][RFC] Introduction of cloud-init configuration in virt-install
On Fri, Jun 21, 2019 at 01:50:51PM -0400, Cole Robinson wrote:
> On 6/21/19 1:34 PM, Daniel P. Berrangé wrote:
> > On Fri, Jun 21, 2019 at 08:18:45PM +0300, Athina Plaskasoviti wrote:
> >> Triggered by:
> >> --install is_cloud=yes ... --import
> >>
> >> Signed-off-by: Athina Plaskasoviti
> >> ---
> >> virt-install| 9 ---
> >> virtinst/cli.py | 2 ++
> >> virtinst/install/cloudinit.py | 41 +
> >> virtinst/install/installer.py | 16 ++-
> >> virtinst/install/installerinject.py | 20 +++---
> >> 5 files changed, 75 insertions(+), 13 deletions(-)
> >> create mode 100644 virtinst/install/cloudinit.py
> >>
> >> diff --git a/virt-install b/virt-install
> >> index ee2b9006..b3608662 100755
> >> --- a/virt-install
> >> +++ b/virt-install
> >> @@ -399,6 +399,7 @@ def build_installer(options, guest, installdata):
> >> install_kernel_args = installdata.kernel_args
> >> install_os = installdata.os
> >> no_install = installdata.no_install
> >> +is_cloud = installdata.is_cloud
> >> if installdata.kernel_args:
> >> if installdata.kernel_args_overwrite:
> >> install_kernel_args = installdata.kernel_args
> >> @@ -417,10 +418,11 @@ def build_installer(options, guest, installdata):
> >> no_install = True
> >> elif options.pxe:
> >> install_bootdev = "network"
> >> +elif options.import_install:
> >> +no_install = True
> >> elif installdata.is_set:
> >> pass
> >> -elif (options.import_install or
> >> - options.xmlonly or
> >> +elif (options.xmlonly or
> >>options.boot):
> >> no_install = True
> >>
> >> @@ -433,7 +435,8 @@ def build_installer(options, guest, installdata):
> >> install_kernel=install_kernel,
> >> install_initrd=install_initrd,
> >> install_kernel_args=install_kernel_args,
> >> -no_install=no_install)
> >> +no_install=no_install,
> >> +is_cloud=is_cloud)
> >>
> >> if options.unattended:
> >> unattended_data = cli.parse_unattended(options.unattended)
> >> diff --git a/virtinst/cli.py b/virtinst/cli.py
> >> index 9a1fe2f6..a2a501a5 100644
> >> --- a/virtinst/cli.py
> >> +++ b/virtinst/cli.py
> >> @@ -1580,6 +1580,7 @@ class ParserInstall(VirtCLIParser):
> >> is_onoff=True)
> >> cls.add_arg("os", "os")
> >> cls.add_arg("no_install", "no_install", is_onoff=True)
> >> +cls.add_arg("is_cloud", "is_cloud", is_onoff=True)
> >>
> >>
> >> class InstallData:
> >> @@ -1592,6 +1593,7 @@ class InstallData:
> >> self.os = None
> >> self.is_set = False
> >> self.no_install = None
> >> +self.is_cloud = None
> >>
> >>
> >> def parse_install(optstr):
> >> diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
> >> new file mode 100644
> >> index ..25b2a79b
> >> --- /dev/null
> >> +++ b/virtinst/install/cloudinit.py
> >> @@ -0,0 +1,41 @@
> >> +import tempfile
> >> +from ..logger import log
> >> +
> >> +
> >> +def create_metadata(scratchdir, hostname=None):
> >> +if hostname:
> >> +instance = hostname
> >> +else:
> >> +hostname = instance = "localhost"
> >> +
> >> +fileobj = tempfile.NamedTemporaryFile(
> >> +prefix="virtinst-", suffix="-metadata",
> >> +dir=scratchdir, delete=False)
> >> +filename = fileobj.name
> >> +
> >> +with open(filename, "w") as f:
> >> +log.debug("Writing instance-id and hostname to file meta-data")
> >> +f.writelines(['instance-id: %s\n' % instance,
> >> +'hostname: %s\n' % hostname])
> >> +return filename
> >
>
> Just a general note, for now we are just trying to straighten out the
> plumbing and get something testable. I'm not planning on pushing
> anything without a broader
Re: [virt-tools-list] [virt-manager PATCH][v2][RFC] Introduction of cloud-init configuration in virt-install
On Fri, Jun 21, 2019 at 08:18:45PM +0300, Athina Plaskasoviti wrote:
> Triggered by:
> --install is_cloud=yes ... --import
>
> Signed-off-by: Athina Plaskasoviti
> ---
> virt-install| 9 ---
> virtinst/cli.py | 2 ++
> virtinst/install/cloudinit.py | 41 +
> virtinst/install/installer.py | 16 ++-
> virtinst/install/installerinject.py | 20 +++---
> 5 files changed, 75 insertions(+), 13 deletions(-)
> create mode 100644 virtinst/install/cloudinit.py
>
> diff --git a/virt-install b/virt-install
> index ee2b9006..b3608662 100755
> --- a/virt-install
> +++ b/virt-install
> @@ -399,6 +399,7 @@ def build_installer(options, guest, installdata):
> install_kernel_args = installdata.kernel_args
> install_os = installdata.os
> no_install = installdata.no_install
> +is_cloud = installdata.is_cloud
> if installdata.kernel_args:
> if installdata.kernel_args_overwrite:
> install_kernel_args = installdata.kernel_args
> @@ -417,10 +418,11 @@ def build_installer(options, guest, installdata):
> no_install = True
> elif options.pxe:
> install_bootdev = "network"
> +elif options.import_install:
> +no_install = True
> elif installdata.is_set:
> pass
> -elif (options.import_install or
> - options.xmlonly or
> +elif (options.xmlonly or
>options.boot):
> no_install = True
>
> @@ -433,7 +435,8 @@ def build_installer(options, guest, installdata):
> install_kernel=install_kernel,
> install_initrd=install_initrd,
> install_kernel_args=install_kernel_args,
> -no_install=no_install)
> +no_install=no_install,
> +is_cloud=is_cloud)
>
> if options.unattended:
> unattended_data = cli.parse_unattended(options.unattended)
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index 9a1fe2f6..a2a501a5 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -1580,6 +1580,7 @@ class ParserInstall(VirtCLIParser):
> is_onoff=True)
> cls.add_arg("os", "os")
> cls.add_arg("no_install", "no_install", is_onoff=True)
> +cls.add_arg("is_cloud", "is_cloud", is_onoff=True)
>
>
> class InstallData:
> @@ -1592,6 +1593,7 @@ class InstallData:
> self.os = None
> self.is_set = False
> self.no_install = None
> +self.is_cloud = None
>
>
> def parse_install(optstr):
> diff --git a/virtinst/install/cloudinit.py b/virtinst/install/cloudinit.py
> new file mode 100644
> index ..25b2a79b
> --- /dev/null
> +++ b/virtinst/install/cloudinit.py
> @@ -0,0 +1,41 @@
> +import tempfile
> +from ..logger import log
> +
> +
> +def create_metadata(scratchdir, hostname=None):
> +if hostname:
> +instance = hostname
> +else:
> +hostname = instance = "localhost"
> +
> +fileobj = tempfile.NamedTemporaryFile(
> +prefix="virtinst-", suffix="-metadata",
> +dir=scratchdir, delete=False)
> +filename = fileobj.name
> +
> +with open(filename, "w") as f:
> +log.debug("Writing instance-id and hostname to file meta-data")
> +f.writelines(['instance-id: %s\n' % instance,
> +'hostname: %s\n' % hostname])
> +return filename
We should probably use the UUID for the newly to-be-created VM as
the instance-id value, and probably not set hostname at all unless
we know what it will end up being.
> +
> +
> +def create_userdata(scratchdir, username=None, password=None):
> +if not password:
> +password = "password"
We must not do this - it is a secret flaw to have a hardcoded
password.
If the user doesn't spply a password, we should simply not
set one.
As default behaviour it is probably more useful to
take $HOME/.ssh/authorized_keys and inject it into
the guest. We should also allow an alterantive file
in case they want different keys in some cases.
> +fileobj = tempfile.NamedTemporaryFile(
> +prefix="virtinst-", suffix="-userdata",
> +dir=scratchdir, delete=False)
> +filename = fileobj.name
> +
> +with open(filename, "w+") as f:
> +f.write("#cloud-config\n")
> +if username:
> +log.debug("Writing username to file user-data")
> +f.write("name: %s\n" % username)
> +log.debug("Writing password and cmd for disabling of cloud-init to
> file user-data")
> +f.writelines(["password: %s\n" % password,
> +"chpasswd: { expire: False }\n", "runcmd:\n",
> +"- [ sudo, touch, /etc/cloud/cloud-init.disabled ]"])
> +return filename
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-
Re: [virt-tools-list] [virt-manager PATCH] installer: Prefer "cdrom" over "floppy"
On Fri, Jun 07, 2019 at 01:32:55PM +0200, Fabiano Fidêncio wrote: > On Fri, Jun 7, 2019 at 12:49 PM Peter Crowther > wrote: > > > > On Fri, 7 Jun 2019 at 11:34, Daniel P. Berrangé wrote: > >> > >> On Fri, Jun 07, 2019 at 11:27:36AM +0100, Peter Crowther wrote: > >> > On Fri, 7 Jun 2019 at 11:18, Daniel P. Berrangé > >> > wrote: > >> > > >> > > On Fri, Jun 07, 2019 at 12:00:56PM +0200, Fabiano Fidêncio wrote: > >> > > > Instead of using "floppy" as the way to perform unattended > >> > > > installations > >> > > > for Windoes, let's prefer using "cdrom" instead. > >> > > > >> > > Aren't there versions of windows which /only/ support "floppy", which > >> > > would require us to preferentially use "cdrom" but fallback to "floppy" > >> > > for older versions ? > > Daniel, > > Older versions of Windows would require floppy. However: > - There are no floppy support for VMs using q35 in some OSes; This doesn't bother me. If some OS vendors want to intentionally cripple floppy support they simply don't get to use OS which need floppy install. > - mtools has been orphaned on Fedora; This is more of a problem. We need devs to be able to test the code and so if Fedora has dropped mtools, the main dev platform of virt-manager is unable to test it. > > With those two things in mind, I'd happily not support those systems > as part of a new feature for virt-install. > Still, if you prefer (and Cole agrees), I'd be okay providing a > fallback code to use floppy as the injection method for Windows and > keep supporting Windows XP. Unless there's a viable alternative to mtools that isn't a huge amount of work, then I think we probably have to drop it. > >> > Not for many, many years. Even Windows 2000 enabled you to burn a CD > >> > with > >> > a winnt.sif answer file on it; I've not checked further back. How far > >> > back > >> > do we wish to support? > >> > >> libosinfo provides install scripts back to XP vintage > >> > > All fine - CD-only unattended installation was well established by that > > point. > > > > Peter, > > Are you sure about this? I couldn't start a unattended installation > using a second cdrom containing the winnit.sif file on it. I think you probably needed to modify the primary cdrom media to contain the sif file. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] installer: Prefer "cdrom" over "floppy"
On Fri, Jun 07, 2019 at 11:27:36AM +0100, Peter Crowther wrote: > On Fri, 7 Jun 2019 at 11:18, Daniel P. Berrangé wrote: > > > On Fri, Jun 07, 2019 at 12:00:56PM +0200, Fabiano Fidêncio wrote: > > > Instead of using "floppy" as the way to perform unattended installations > > > for Windoes, let's prefer using "cdrom" instead. > > > > Aren't there versions of windows which /only/ support "floppy", which > > would require us to preferentially use "cdrom" but fallback to "floppy" > > for older versions ? > > > > Not for many, many years. Even Windows 2000 enabled you to burn a CD with > a winnt.sif answer file on it; I've not checked further back. How far back > do we wish to support? libosinfo provides install scripts back to XP vintage 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH] installer: Prefer "cdrom" over "floppy"
On Fri, Jun 07, 2019 at 12:00:56PM +0200, Fabiano Fidêncio wrote:
> Instead of using "floppy" as the way to perform unattended installations
> for Windoes, let's prefer using "cdrom" instead.
Aren't there versions of windows which /only/ support "floppy", which
would require us to preferentially use "cdrom" but fallback to "floppy"
for older versions ?
> The main reasons behind this change are:
> - VMs using q35 may have floppy device disabled in some systems;
> - We can drop mstools dependency;
>
> Generating the ISO depends on genisofs, tho. However, it's not a big
> deal as genisofs is already a virt-manager dependency.
>
> Signed-off-by: Fabiano Fidêncio
> ---
> virt-manager.spec.in | 5
> virtinst/{floppyinject.py => cdrominject.py} | 26 +++--
> virtinst/installer.py| 30 ++--
> virtinst/unattended.py | 2 +-
> 4 files changed, 33 insertions(+), 30 deletions(-)
> rename virtinst/{floppyinject.py => cdrominject.py} (55%)
>
> diff --git a/virt-manager.spec.in b/virt-manager.spec.in
> index b5f6979b..43e0c10b 100644
> --- a/virt-manager.spec.in
> +++ b/virt-manager.spec.in
> @@ -48,11 +48,6 @@ Requires: dconf
> # no ambiguity.
> Requires: vte291
>
> -# Those two dependencies are needed in order to support unattended
> -# installation for Windows guests.
> -Requires: dosfstools
> -Requires: mtools
> -
> # Weak dependencies for the common virt-manager usecase
> Recommends: (libvirt-daemon-kvm or libvirt-daemon-qemu)
> Recommends: libvirt-daemon-config-network
> diff --git a/virtinst/floppyinject.py b/virtinst/cdrominject.py
> similarity index 55%
> rename from virtinst/floppyinject.py
> rename to virtinst/cdrominject.py
> index 05f91ee0..9dc6e20a 100644
> --- a/virtinst/floppyinject.py
> +++ b/virtinst/cdrominject.py
> @@ -6,11 +6,12 @@
>
> import logging
> import os
> +import shutil
> import subprocess
> import tempfile
>
>
> -def perform_floppy_injections(injections, scratchdir):
> +def perform_cdrom_injections(injections, scratchdir):
> """
> Insert files into the root directory of a floppy
> """
> @@ -20,17 +21,24 @@ def perform_floppy_injections(injections, scratchdir):
> tempdir = tempfile.mkdtemp(dir=scratchdir)
> os.chmod(tempdir, 0o775)
>
> -img = os.path.join(tempdir, "unattended.img")
> +tempfiles = []
> +iso = os.path.join(tempdir, "unattended.iso")
> +for filename in injections:
> +shutil.copy(filename, tempdir)
> +
> +tempfiles = os.listdir(tempdir)
>
> -cmd = ["mkfs.msdos", "-C", img, "1440"]
> +cmd = ["mkisofs",
> + "-o", iso,
> + "-J",
> + "-input-charset", "utf8",
> + "-rational-rock",
> + tempdir]
> logging.debug("Running mkisofs: %s", cmd)
> output = subprocess.check_output(cmd)
> logging.debug("cmd output: %s", output)
>
> -for filename in injections:
> -logging.debug("Copying %s to the floppy.", filename)
> -cmd = ["mcopy", "-i", img, filename, "::"]
> -output = subprocess.check_output(cmd)
> -logging.debug("cmd output: %s", output)
> +for f in tempfiles:
> +os.unlink(os.path.join(tempdir, f))
>
> -return img
> +return iso
> diff --git a/virtinst/installer.py b/virtinst/installer.py
> index 45caf930..e6524708 100644
> --- a/virtinst/installer.py
> +++ b/virtinst/installer.py
> @@ -15,7 +15,7 @@ from .devices import DeviceDisk
> from .domain import DomainOs
> from .osdict import OSDB, OsMedia
> from .installertreemedia import InstallerTreeMedia
> -from .floppyinject import perform_floppy_injections
> +from .cdrominject import perform_cdrom_injections
> from . import unattended
> from . import util
>
> @@ -51,7 +51,7 @@ class Installer(object):
> self._install_kernel = None
> self._install_initrd = None
> self._install_cdrom_device_added = False
> -self._install_floppy_device = False
> +self._unattended_install_cdrom_device = None
> self._unattended_files = []
> self._defaults_are_set = False
> self._unattended_data = None
> @@ -119,25 +119,25 @@ class Installer(object):
> disk.sync_path_props()
> break
>
> -def _add_install_floppy_device(self, guest, location):
> -if self._install_floppy_device:
> +def _add_unattended_install_cdrom_device(self, guest, location):
> +if self._unattended_install_cdrom_device:
> return
> dev = DeviceDisk(self.conn)
> -dev.device = dev.DEVICE_FLOPPY
> +dev.device = dev.DEVICE_CDROM
> dev.path = location
> dev.sync_path_props()
> dev.validate()
> dev.set_defaults(guest)
> -self._install_floppy_device = dev
> -guest.add_device(self._install_floppy_device)
> +self._unattended_install_cdrom_device = dev
> +
[virt-tools-list] [virt-manager PATCH] domcapabilities: add md-clear to automatically enabled security features
The bit is set when microcode provides the mechanism to invoke a flush of various exploitable CPU buffers by invoking the VERW instruction. CVE-2018-12126, CVE-2018-12127, CVE-2018-12130, CVE-2019-11091 Signed-off-by: Daniel P. Berrangé --- virtinst/domcapabilities.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py index 8993822e..acc91f81 100644 --- a/virtinst/domcapabilities.py +++ b/virtinst/domcapabilities.py @@ -281,7 +281,8 @@ class DomainCapabilities(XMLBuilder): 'spec-ctrl', 'ssbd', 'ibpb', -'virt-ssbd'] +'virt-ssbd', +'md-clear'] if self._features: return self._features -- 2.21.0 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 5/5] virt-manager: add new checkbox to control CPU security features
On Wed, Apr 03, 2019 at 03:52:51PM +0200, Pavel Hrdina wrote: > By default we copy CPU security features to the guest if specific CPU > model is selected. However, this may break migration and will affect > performance of the guest. This adds an option to disable this default > behavior. > > The checkbox is clickable only on x86 and only on host where we can > detect any CPU security features, otherwise a tooltip is set to notify > users that there is nothing to copy. > > Signed-off-by: Pavel Hrdina > --- > ui/details.ui | 25 + > virtManager/details.py | 21 + > virtManager/domain.py | 5 +++-- > virtinst/domain/cpu.py | 30 ++ > 4 files changed, 79 insertions(+), 2 deletions(-) > > diff --git a/ui/details.ui b/ui/details.ui > index 8b3f939e..2ae1c104 100644 > --- a/ui/details.ui > +++ b/ui/details.ui > @@ -2122,6 +2122,31 @@ > name="top_attach">1 > > > + > + id="cpu-secure-label"> > + name="visible">True > + name="can_focus">False > + translatable="yes">Copy host security features: I'd probably change the label to "Enable known CPU security flaw mitigations" Reviewed-by: Daniel P. Berrangé 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 4/5] domcapabilities: add caching of CPU security features
On Wed, Apr 03, 2019 at 03:52:50PM +0200, Pavel Hrdina wrote: > We will call this function multiple times so it makes sense to cache the > result so we don't have to call libvirt APIs every time we will check > what security features are available on the host. > > Signed-off-by: Pavel Hrdina > --- > virtinst/domcapabilities.py | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) Arguably you can cache the entire capabilities API call for virt-install since it won't change in the lifetime if a short virt-install invokation and even if it does change you likely want virt-install to have a consistent view of capabilities throughout its execution. For virt-manager though being more dynamic is possibly more desirable given its long life. Reviewed-by: Daniel P. Berrangé 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 3/5] cli: introduce CPU secure parameter
On Thu, Apr 04, 2019 at 10:14:21AM +0100, Daniel P. Berrangé wrote: > On Wed, Apr 03, 2019 at 03:52:49PM +0200, Pavel Hrdina wrote: > > This will allow users to override the default behavior of virt-install > > which copies CPU security features available on the host to the guest > > XML if specific CPU model is configured. > > > > Signed-off-by: Pavel Hrdina > > --- > > man/virt-install.pod | 8 +- > > .../compare/virt-install-cpu-disable-sec.xml | 93 +++ > > tests/clitest.py | 1 + > > virtinst/cli.py | 1 + > > virtinst/domain/cpu.py| 7 +- > > 5 files changed, 108 insertions(+), 2 deletions(-) > > create mode 100644 > > tests/cli-test-xml/compare/virt-install-cpu-disable-sec.xml > > > > diff --git a/man/virt-install.pod b/man/virt-install.pod > > index 8407e795..18d44808 100644 > > --- a/man/virt-install.pod > > +++ b/man/virt-install.pod > > @@ -216,7 +216,13 @@ required value is MODEL, which is a valid CPU model as > > known to libvirt. > > > > Libvirt's feature policy values force, require, optional, disable, or > > forbid, > > or with the shorthand '+feature' and '-feature', which equal > > 'force=feature' > > -and 'disable=feature' respectively > > +and 'disable=feature' respectively. > > + > > +If exact CPU model is specified virt-install will automatically copy CPU > > +security features available on the host to mitigate recent CPU CVEs. > > I'd tweak it slightly to > > s/security features/features/ > > s/CPU CVEs/CPU speculative execution side channel security vulnerabilities./ > > > +This however will have some impact on performance and will break migration > > +to hosts without security patches. In order to turn off this default > > behavior > > +there is a B parameter. Possible values are I and I. > > At the end, add > > , with I as the default. It is highly recommended to leave this > enabled and ensure all virtualization hosts have fully up to date > microcode, kernel & virtualization software installed. With those changes applied Reviewed-by: Daniel P. Berrangé 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 3/5] cli: introduce CPU secure parameter
On Wed, Apr 03, 2019 at 03:52:49PM +0200, Pavel Hrdina wrote: > This will allow users to override the default behavior of virt-install > which copies CPU security features available on the host to the guest > XML if specific CPU model is configured. > > Signed-off-by: Pavel Hrdina > --- > man/virt-install.pod | 8 +- > .../compare/virt-install-cpu-disable-sec.xml | 93 +++ > tests/clitest.py | 1 + > virtinst/cli.py | 1 + > virtinst/domain/cpu.py| 7 +- > 5 files changed, 108 insertions(+), 2 deletions(-) > create mode 100644 > tests/cli-test-xml/compare/virt-install-cpu-disable-sec.xml > > diff --git a/man/virt-install.pod b/man/virt-install.pod > index 8407e795..18d44808 100644 > --- a/man/virt-install.pod > +++ b/man/virt-install.pod > @@ -216,7 +216,13 @@ required value is MODEL, which is a valid CPU model as > known to libvirt. > > Libvirt's feature policy values force, require, optional, disable, or forbid, > or with the shorthand '+feature' and '-feature', which equal 'force=feature' > -and 'disable=feature' respectively > +and 'disable=feature' respectively. > + > +If exact CPU model is specified virt-install will automatically copy CPU > +security features available on the host to mitigate recent CPU CVEs. I'd tweak it slightly to s/security features/features/ s/CPU CVEs/CPU speculative execution side channel security vulnerabilities./ > +This however will have some impact on performance and will break migration > +to hosts without security patches. In order to turn off this default > behavior > +there is a B parameter. Possible values are I and I. At the end, add , with I as the default. It is highly recommended to leave this enabled and ensure all virtualization hosts have fully up to date microcode, kernel & virtualization software installed. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 1/5] domcapabilities: remove recommended CPU features from security features
On Wed, Apr 03, 2019 at 03:52:47PM +0200, Pavel Hrdina wrote: > These features are only recommended to be enabled since they improve > performance of the VMs if security features are enabled. > > Signed-off-by: Pavel Hrdina > --- > tests/cli-test-xml/compare/virt-install-qemu-plain.xml | 2 -- > .../compare/virt-install-singleton-config-2.xml | 4 > virtinst/domcapabilities.py | 6 +- > 3 files changed, 1 insertion(+), 11 deletions(-) > diff --git a/virtinst/domcapabilities.py b/virtinst/domcapabilities.py > index d1b0f4ed..72844512 100644 > --- a/virtinst/domcapabilities.py > +++ b/virtinst/domcapabilities.py > @@ -274,14 +274,10 @@ class DomainCapabilities(XMLBuilder): > > def get_cpu_security_features(self): > sec_features = [ > -'pcid', > 'spec-ctrl', > 'ssbd', > -'pdpe1gb', > 'ibpb', > -'virt-ssbd', > -'amd-ssbd', > -'amd-no-ssb'] > +'virt-ssbd'] This all makes sense - rationale for each removed one is: pcid is a very useful perf feature, but missing in some silicon so not portable. pdpe1gb lets the guest use 1 GB pages which is good for perf but again not all silicon can do it amd-ssbd is a security feature which fixes the same SSBD flaws as the virt-ssbd feature does. virt-ssbd is usable across all CPU models affected by SSBD, while amd-ssbd is only available in very new silicon. So virt-ssbd is the bette rchoice. amd-no-ssb just indicates that the CPU is not affected by SSBD, so not critical to expose. I expect a future named CPU model will include that where appropriate. Reviewed-by: Daniel P. Berrangé 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 2/5] domcapabilities: fix typo in function name
On Wed, Apr 03, 2019 at 03:52:48PM +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > virtinst/domcapabilities.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 0/5] CPU security features improvements
On Thu, Apr 04, 2019 at 10:00:21AM +0100, Peter Crowther wrote: > On Thu, 4 Apr 2019 at 09:15, Daniel P. Berrangé wrote: > > > On Wed, Apr 03, 2019 at 07:49:48PM -0400, Cole Robinson wrote: > > I > > think it is reasonable to assume that if the user has upgraded the > > microcode on some hosts they will have done it on all hosts. > > > Don't rely on it - certainly in the cases of the smaller organisations I > work with / audit, patching can be politely described as "haphazard". Even > in the large public-sector organisations I work with, there's almost never > money for test rigs that have the same hardware as the live hosts. It's > not uncommon for a live host to be the "guinea pig" receiving new microcode > before others, then returned to live service. > > The real world is not tidy :-(. Sure, that's why this series proposes a command line option to let the user disable the security features if they are in a messy situations. > > If they > > have not upgraded on all hosts, I still think it is sensible to > > apply the security mitigations on the hosts which have been upgraded > > unless the user explicitly says to use the insecure mode. > > > > Agree. > > We should do the right thing out of the box to enable the > > security mitigations. The fact that virt-intsall doesn't do this for > > named CPU models is arguably worthy of filing a CVE against virt-install > > itself. > > > > Agree. > > > > > Regards, > > Daniel > > > > - Peter 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-manager PATCH 0/5] CPU security features improvements
On Wed, Apr 03, 2019 at 07:49:48PM -0400, Cole Robinson wrote: > ccing danpb > > On 4/3/19 9:52 AM, Pavel Hrdina wrote: > > Pavel Hrdina (5): > > domcapabilities: remove recommended CPU features from security > > features > > domcapabilities: fix typo in function name > > cli: introduce CPU secure parameter > > domcapabilities: add caching of CPU security features > > virt-manager: add new checkbox to control CPU security features > > > > So the bug spawning the original work is: > > https://bugzilla.redhat.com/show_bug.cgi?id=1582667 > > Some CPU model names lack security critical CPU flags. Example > Opteron_G5 lacks ibpb. Even if the user is running on an Opteron_G5 host > CPU which has ibpb, if they select Opteron_G5 the guest doesn't see it. > > In that case, the existing patches in git will check to see if the host > supports ibpb, and if so, manually specify that in the > config. So now use of --cpu Opteron_G5 is secure by default. > > But the problem is this creates a migration compat issue: selecting > --cpu Opteron_G5 means different things for different hosts. So these > patches basically make the 'secure' additions the default > behavior, but gives users the option to turn it off on the cli and > virt-manager UI. You are correct about migration compat in the general case, but I think it is reasonable to assume that if the user has upgraded the microcode on some hosts they will have done it on all hosts. If they have not upgraded on all hosts, I still think it is sensible to apply the security mitigations on the hosts which have been upgraded unless the user explicitly says to use the insecure mode. > Taking a step back, I don't really get what the usecase is for users to > be selecting manual cpu model names with virt-manager or virt-install. I > understand the migration compat thing, if you need to generate a > baseline CPU that works for migrating a machine across multiple hosts, > but in practice that seems like pretty advanced usage for people using > virt-manager or even virt-install, and if they are using virt-install > they have all the tools at their disposal to generate a full + secure > config if they want. I think using a explicit CPU model is quite a reasonable thing todo. It isn't solely live migration it helps. Even save/restore where you restore the image on 2nd machine wants a portable CPU model. It may not be the out of the box common case, but I don't think we should write it off as "advanced" usage and require people to take extra steps to figure out which security flags they need to set themselves. > Anyways, since the current code requires the user to explicitly opt in > to choosing a manual CPU model name, what if instead we just warn them? For these hardware security flaws I don't think it is acceptable to merely warn the user. We should do the right thing out of the box to enable the security mitigations. The fact that virt-intsall doesn't do this for named CPU models is arguably worthy of filing a CVE against virt-install itself. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-viewer 8.0?
On Fri, Mar 29, 2019 at 06:43:34PM +0300, Hetz Ben Hamo wrote: > Oh thanks. > Any chance for a Fedora 29 build also please? :) I generally avoid sending rebases into existing Fedora releases. The time between Fedora releases is short enough that users don't have to wait long to get the new versions & I prefer to avoid risk of creating regressions in existing releases. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] virt-viewer 8.0?
On Fri, Mar 29, 2019 at 05:38:29PM +0300, Hetz Ben Hamo wrote: > Hi, > > I see virt-viewer is available as version 8.0 on the home page. It's > available for Windows as well as the source. > > When will it be available as a binary package for linux distributions? I forgot to update Fedora, so I've submitted it as an update for rawhide and Fedora 30 now. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer PATCH 01/14] po: provide custom make rules for po file management
On Mon, Mar 11, 2019 at 12:05:25PM +0100, Christophe Fergeau wrote: > Hey, > > On Thu, Mar 07, 2019 at 02:42:45PM +0000, Daniel P. Berrangé wrote: > > After investigating in more detail it seems you can provide custom rules > > for XML files by setting GETTEXTDATADIR, so I can support mime files with > > this extra diff: > > Ah indeed, nice finding. Seeing what is shipped in > https://git.savannah.gnu.org/cgit/gettext.git/tree/gettext-tools/its > (gsettings, glade, ... rules), this mime rule could probably go there as > well. > > > > > diff --git a/data/Makefile.am b/data/Makefile.am > > index 0e50f3d..d4089be 100644 > > --- a/data/Makefile.am > > +++ b/data/Makefile.am > > @@ -4,6 +4,8 @@ MANUFACTURER = Virt Manager Project > > > > EXTRA_DIST = \ > > virt-viewer.wxs.in \ > > + gettext/its/mime.its\ > > + gettext/its/mime.loc\ > > $(NULL) > > > > # this make sure those files are regenerated when they change > > @@ -71,8 +73,11 @@ desktop_DATA = $(DESKTOPFILES:.desktop.in=.desktop) > > %.desktop: %.desktop.in > > $(AM_V_GEN)$(MSGFMT) --desktop --template $< -d $(top_srcdir)/po -o > > $@ > > > > +MIMEFILES = virt-viewer-mime.xml.in > > mimedir = $(datadir)/mime/packages > > mime_DATA = virt-viewer-mime.xml > > +%-mime.xml: %-mime.xml.in > > + $(AM_V_GEN)GETTEXTDATADIR=$(srcdir)/gettext $(MSGFMT) --xml > > --template $< -d $(top_srcdir)/po -o $@ > > Something I just noticed (but which is related to the initial patch > rather than this one) is that the .xml.in -> .xml rule is no longer > triggered when one of the .po file changes (same for the .desktop file). > I tried adding $(wildcard $(top_srcdir)/po/*.po) as a dependency, and as > far as I can tell, this is giving the behaviour I want. Yes, I can see the original rule had such a dep $ grep XML_RULE /usr/share/aclocal/intltool.m4 INTLTOOL_XML_RULE='%.xml: %.xml.in $(INTLTOOL_MERGE) $(wildcard $(top_srcdir)/po/*.po) I'll add this dep before pushing. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer PATCH 01/14] po: provide custom make rules for po file management
On Tue, Mar 05, 2019 at 11:51:01AM +0100, Christophe Fergeau wrote: > On Tue, Mar 05, 2019 at 10:27:07AM +0000, Daniel P. Berrangé wrote: > > On Tue, Mar 05, 2019 at 11:23:34AM +0100, Christophe Fergeau wrote: > > > On Wed, Feb 20, 2019 at 05:50:52PM +, Daniel P. Berrangé wrote: > > > > Historically we have relied on intltool to install a standard > > > > po/Makefile.in.in which has very limited scope for customization. > > > > intltool is deprecated in favour of standard gettextize tools, > > > > but these share the same disadvantages. > > > > > > > > Writing make rules for po file management is no more difficult > > > > than any other rules libvirt-glib has, so stop using intltool > > > > > > s/libvirt-glib/virt-viewer > > > > > > > and don't use gettextize ether. > > > > > > > Signed-off-by: Daniel P. Berrangé > > > > > > > diff --git a/data/Makefile.am b/data/Makefile.am > > > > index 1f6c8bf..0e50f3d 100644 > > > > --- a/data/Makefile.am > > > > +++ b/data/Makefile.am > > > > @@ -68,17 +68,17 @@ else #!WIN32 > > > > desktopdir = $(datadir)/applications > > > > DESKTOPFILES = remote-viewer.desktop.in > > > > desktop_DATA = $(DESKTOPFILES:.desktop.in=.desktop) > > > > -@INTLTOOL_DESKTOP_RULE@ > > > > +%.desktop: %.desktop.in > > > > + $(AM_V_GEN)$(MSGFMT) --desktop --template $< -d > > > > $(top_srcdir)/po -o $@ > > > > > > > > -MIMEFILES = virt-viewer-mime.xml.in > > > > mimedir = $(datadir)/mime/packages > > > > mime_DATA = virt-viewer-mime.xml > > > > -@INTLTOOL_XML_RULE@ > > > > > > Why drop translations from the mime file? > > > > I don't see any other existing mime files translated, and the gettext > > tools don't recognise the file format AFAICT, so I didn't look like > > this was really needed in the first place. > > On my system, grep -l xml:lang /usr/share/mime/packages/* gives a few > hits, including libreoffice, scribus and the main shared-mime-info file. After investigating in more detail it seems you can provide custom rules for XML files by setting GETTEXTDATADIR, so I can support mime files with this extra diff: diff --git a/data/Makefile.am b/data/Makefile.am index 0e50f3d..d4089be 100644 --- a/data/Makefile.am +++ b/data/Makefile.am @@ -4,6 +4,8 @@ MANUFACTURER = Virt Manager Project EXTRA_DIST = \ virt-viewer.wxs.in \ + gettext/its/mime.its\ + gettext/its/mime.loc\ $(NULL) # this make sure those files are regenerated when they change @@ -71,8 +73,11 @@ desktop_DATA = $(DESKTOPFILES:.desktop.in=.desktop) %.desktop: %.desktop.in $(AM_V_GEN)$(MSGFMT) --desktop --template $< -d $(top_srcdir)/po -o $@ +MIMEFILES = virt-viewer-mime.xml.in mimedir = $(datadir)/mime/packages mime_DATA = virt-viewer-mime.xml +%-mime.xml: %-mime.xml.in + $(AM_V_GEN)GETTEXTDATADIR=$(srcdir)/gettext $(MSGFMT) --xml --template $< -d $(top_srcdir)/po -o $@ appdatadir = $(datadir)/appdata APPDATAFILES = remote-viewer.appdata.xml.in @@ -92,8 +97,8 @@ if ENABLE_UPDATE_MIMEDB $(UPDATE_MIME_DATABASE) "$(DESTDIR)$(datadir)/mime"; endif -CLEANFILES += $(desktop_DATA) $(appdata_DATA) -EXTRA_DIST += $(mime_DATA) $(DESKTOPFILES) $(APPDATAFILES) +CLEANFILES += $(mime_DATA) $(desktop_DATA) $(appdata_DATA) +EXTRA_DIST += $(MIMEFILES) $(DESKTOPFILES) $(APPDATAFILES) endif diff --git a/data/gettext/its/mime.its b/data/gettext/its/mime.its new file mode 100644 index 000..dec8d13 --- /dev/null +++ b/data/gettext/its/mime.its @@ -0,0 +1,6 @@ + +http://www.w3.org/2005/11/its; + version="2.0"> + + + diff --git a/data/gettext/its/mime.loc b/data/gettext/its/mime.loc new file mode 100644 index 000..a34fe8d --- /dev/null +++ b/data/gettext/its/mime.loc @@ -0,0 +1,6 @@ + + + + + + diff --git a/data/virt-viewer-mime.xml b/data/virt-viewer-mime.xml.in similarity index 100% rename from data/virt-viewer-mime.xml rename to data/virt-viewer-mime.xml.in 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer PATCH 00/14] po: improve translation handling
On Tue, Mar 05, 2019 at 11:24:49AM +0100, Christophe Fergeau wrote: > A few comments on the first patch, for the rest of the series, > > Reviewed-by: Christophe Fergeau > > Did you get feedback from people involved with translations regarding > these changes? No, but in general case it will have no impact on them. We did this in libvirt a while back & have seen no complaints either. We outsourced translation to Zanata, and the people doing translations in Zanata see zero changes in content presented to them, as zanata unpacks .po files into its own DB & reconciles with the .pot. So all the metadata we discard from the .po files was never used by Zanata in the first place, as it gets it all from the pot. In the unlikely event anyone was using the .po files in git, the metadata they were relying on was always outdated & thus misleading and/or useless. They would have to re-generate the .po file to get accurate source locations. With the new system we discarded all the inaccurate metadata, and people can fully up2date metadata by running 'make update-po'. IOW, this is better for any one using .po files directly, as the info they see is now actually accurate. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer PATCH] Add a git-publish configuration file
On Tue, Feb 19, 2019 at 03:10:33PM +, Daniel P. Berrangé wrote: > On Tue, Feb 19, 2019 at 04:05:00PM +0100, Marc-André Lureau wrote: > > Hi > > On Tue, Feb 19, 2019 at 4:03 PM Daniel P. Berrangé > > wrote: > > > > > > Signed-off-by: Daniel P. Berrangé > > > --- > > > .gitpublish | 4 > > > 1 file changed, 4 insertions(+) > > > create mode 100644 .gitpublish > > > > > > diff --git a/.gitpublish b/.gitpublish > > > new file mode 100644 > > > index 000..bf82571 > > > --- /dev/null > > > +++ b/.gitpublish > > > @@ -0,0 +1,4 @@ > > > +[gitpublishprofile "default"] > > > +base = master > > > +to = virt-tools-list@redhat.com > > > +prefix = virt-viewer PATCH > > > > In general, I believe "PATCH virt-viewer" order is more common. > > When we did this for libvirt, the exact opposite was asserted, hence > we used "$module PATCH". I think it does make sense to have the > "PATCH" word next to the patch sequence numbers too. I'd like to be > consistent with libvirt in this area in any case. virt-manager patches which also go to this list have the same format as I proposed here, so I've pushed this as is, for consistency between projects 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer PATCH 00/14] po: improve translation handling
ping... On Wed, Feb 20, 2019 at 05:50:51PM +, Daniel P. Berrangé wrote: > This applies the same improvements previously done in libvirt: > > https://www.redhat.com/archives/libvir-list/2018-April/msg01004.html > > https://www.berrange.com/posts/2018/11/29/improved-translation-po-file-handling-by-ditching-gettext-autotools-integration/ > > The key problems with our current approach are: > > - The pot & po files stored in GIT contain huge set of >annotations about source file names & line numbers. >These are out of date as soon as a change is commited >to git following a translation refresh. This makes >diffs impossible to meaningfully review, as they are >98% noise, 2% signal. > > - The po file messages are sorted by source location, >so when we move code between files, or rename files, >the po file message order changes for no good reason. >This makes diffs even more impossible to review. > > - The po files contain entries for all messages even >if most have no translation, bloating size of po/ >data stored in git > > - Whenever 'make dist' is run, it alters all the pot >and po files, so developers need to then reset their >content to match git HEAD manually. This is caused >by having auto-generated content (source file locations) >mixed in with the static content (the actual translated >strings) > > After this series, we only minimized po files in git, with > the redundated & outdated source locations info stripped. > This stripped info is re-added automatically during build > to create the real .po files, that we distribute, and/or > upload to translators in Zanata. > > As a result the po directory is smaller in size, and > when refreshing from Zanata, we have git commits that > clearly show *only* the altered translations, nothing > else. The importance of this cannot be overstated - by > having these clear diffs when doing this change in libvirt, > I discovered a serious bug in the Zanata client that has > been screwing up translations in every project that uses > Zanata by adding bogus "fuzzy" annotations. > > The particularly attractive statistic: > > 193 files changed, 11904 insertions(+), 80334 deletions(-) > > Daniel P. Berrangé (14): > po: provide custom make rules for po file management > po: remove language list from zanata configuration > po: add rules for integration with zanata > po: minimize & canonicalize translations stored in git > po: minimize af am anp ar as ast bal be bg > po: minimize bn_IN bn bo br brx bs ca cs cy da > po: minimize de_CH de el en_GB eo es et eu fa fi > po: minimize fr gl gu he hi hr hu ia > po: minimize id ilo is it ja ka kk km kn ko > po: minimize kw_GB kw@kkcor kw kw@uccor ky lt lv mai mk ml > po: minimize mn mr ms nb nds ne nl nn nso > po: minimize or pa pl pt_BR pt ro ru si sk sl > po: minimize sq sr@latin sr sv ta te tg th tr tw > po: minimize uk ur vi wba yo zh_CN zh_HK zh_TW zu > > Makefile.am |7 - > autogen.sh|1 - > build-aux/minimize-po.pl | 37 + > configure.ac |6 +- > data/Makefile.am | 12 +- > data/remote-viewer.appdata.xml.in | 12 +- > data/remote-viewer.desktop.in |4 +- > ...iewer-mime.xml.in => virt-viewer-mime.xml} |0 > m4/virt-nls.m4| 45 + > mingw-virt-viewer.spec.in |2 +- > po/Makefile.am| 115 ++ > po/POTFILES | 26 + > po/POTFILES.in| 27 - > po/README.md | 76 ++ > po/af.mini.po | 19 + > po/af.po | 864 - > po/am.mini.po | 19 + > po/am.po | 864 - > po/anp.mini.po| 19 + > po/anp.po | 864 - > po/ar.mini.po | 20 + > po/ar.po | 869 - > po/as.mini.po | 327 + > po/as.po | 884 - > po/ast.mini.po| 19 + > po/ast.po | 864 - > po/bal.mini.po| 19 + > po/bal.po
Re: [virt-tools-list] virt manager downloads not working
On Tue, Feb 26, 2019 at 01:07:15PM -0800, Dusty Humphries wrote: > Hi, > > I am attempting to download virt-viewer and virt-manager from > virtualize-manager.org but the downloads are not working for both. Is > someone on this list able to correct the issue please? That website address is wrong. You need this: https://virt-manager.org/download/ I've confirmed that downloads work from there 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer PATCH 04/14] po: minimize & canonicalize translations stored in git
Similar to the virt-viewer.pot, .po files contain line numbers and file
names identifying where in the source a translatable string comes from.
The source locations in the .po files are thrown away and replaced with
content from the virt-viewer.pot whenever msgmerge is run, so this is not
precious information that needs to be stored in git.
When msgmerge processes a .po file, it will add in any msgids from the
virt-viewer.pot that were not already present. Thus, if a particular msgid
currently has no translation, it can be considered redundant and again
does not need storing in git.
When msgmerge processes a .po file and can't find an exact existing
translation match, it will try todo fuzzy matching instead, marking such
entries with a "# fuzzy" comment to alert the translator to take a
look and either discard, edit or accept the match. Looking at the
existing fuzzy matches in .po files shows that the quality is awful,
with many having a completely different set of printf format specifiers
between the msgid and fuzzy msgstr entry. Fortunately when msgfmt
generates the .gmo, the fuzzy entries are all ignored anyway. The fuzzy
entries could be useful to translators if they were working on the .po
files directly from git, but Virt-Viewer outsourced translation to the
Fedora Zanata system, so keeping fuzzy matches in git is not much help.
Finally, by default msgids are sorted based on source location. Thus, if
a bit of code with translatable text is moved from one file to another,
it may shift around in the .po file, despite the msgid not itself changing.
If the msgids were sorted alphabetically, the .po files would have
stable ordering when code is refactored.
This patch takes advantage of the above observations to canonicalize
and minimize the content stored for .po files in git. Instead of storing
the real .po files, we now store .mini.po files.
The .mini.po files are the same file format as .po files, but have no
source location comments, are sorted alphabetically, and all fuzzy
msgstrs and msgids with no translation are discarded. This cuts the size
of content in the po directory.
Users working from a virt-viewer git checkout who need the full .po files
can run "make update-po", which merges the virt-viewer.pot and .mini.po
file to create a .po file containing all the content previously stored
in git.
Conversely if a full .po file has been modified, for example, by
downloading new content from Zanata, the .mini.po files can be updated
by running "make update-mini-po". The resulting diffs of the .mini.po
file will clearly show the changed translations without any of the noise
that previously obscured content. Being able to see content changes
clearly actually identified a bug in the zanata python client where it
was adding bogus "fuzzy" annotations to many messages:
https://bugzilla.redhat.com/show_bug.cgi?id=1564497
Users working from virt-viewer releases should not see any difference in
behaviour, since the tarballs only contain the full .po files, not the
.mini.po files.
As an added benefit, generating tarballs with "make dist", will no
longer cause creation of dirty files in git, since it won't touch the
.mini.po files, only the .po files which are no longer kept in git.
The languages are minimized in the following commit since it is a
large mechanical process.
Signed-off-by: Daniel P. Berrangé
---
build-aux/minimize-po.pl | 37 +
po/Makefile.am | 30 +++--
po/README.md | 58 +---
3 files changed, 101 insertions(+), 24 deletions(-)
create mode 100755 build-aux/minimize-po.pl
diff --git a/build-aux/minimize-po.pl b/build-aux/minimize-po.pl
new file mode 100755
index 000..497533a
--- /dev/null
+++ b/build-aux/minimize-po.pl
@@ -0,0 +1,37 @@
+#!/usr/bin/perl
+
+my @block;
+my $msgstr = 0;
+my $empty = 0;
+my $unused = 0;
+my $fuzzy = 0;
+while (<>) {
+if (/^$/) {
+if (!$empty && !$unused && !$fuzzy) {
+print @block;
+}
+@block = ();
+$msgstr = 0;
+$fuzzy = 0;
+push @block, $_;
+} else {
+if (/^msgstr/) {
+$msgstr = 1;
+$empty = 1;
+}
+if (/^#.*fuzzy/) {
+$fuzzy = 1;
+}
+if (/^#~ msgstr/) {
+$unused = 1;
+}
+if ($msgstr && /".+"/) {
+$empty = 0;
+}
+push @block, $_;
+}
+}
+
+if (@block && !$empty && !$unused) {
+print @block;
+}
diff --git a/po/Makefile.am b/po/Makefile.am
index f32537d..4950cbe 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -2,7 +2,7 @@ DOMAIN = $(PACKAGE_NAME)
COPYRIGHT_HOLDER = The Libvirt authors
MSGID_BUGS_ADDRESS = https://libvirt.org/bugs.html
-MAINTAINERCLEANFILES = $(GMOFILES) $(POTFILE)
+MAINTAINERCLEANFILES = $(GMOFILES) $(POTFILE) $(POFILES)
[virt-tools-list] [virt-viewer PATCH 03/14] po: add rules for integration with zanata
Add rules to handle pushing virt-viewer.pot to zanata, and refreshing .po files with new content from zanata. Signed-off-by: Daniel P. Berrangé --- po/Makefile.am | 8 1 file changed, 8 insertions(+) diff --git a/po/Makefile.am b/po/Makefile.am index c5f7c36..f32537d 100644 --- a/po/Makefile.am +++ b/po/Makefile.am @@ -63,6 +63,14 @@ update-po: $(POFILES) update-gmo: $(GMOFILES) +push-pot: $(POTFILE) + zanata push --push-type=source + +pull-po: $(POTFILE) + zanata pull --create-skeletons + $(MAKE) update-po + $(MAKE) update-gmo + $(POTFILE): POTFILES $(POTFILE_DEPS) $(XGETTEXT) -o $@-t $(XGETTEXT_ARGS) \ --files-from=$(abs_srcdir)/POTFILES -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer PATCH 02/14] po: remove language list from zanata configuration
The element in zanata.xml is no longer relevant as this info is recorded server side. Signed-off-by: Daniel P. Berrangé --- po/zanata.xml | 99 --- 1 file changed, 99 deletions(-) diff --git a/po/zanata.xml b/po/zanata.xml index 63c682e..09396d3 100644 --- a/po/zanata.xml +++ b/po/zanata.xml @@ -4,103 +4,4 @@ virt-viewer master gettext - - -sq -ar -as -ast -bal -eu -bn -bn-IN -brx -bs -br -bg -ca -zh-CN -zh-HK -zh-TW -kw -kw-GB -cs -da -nl -en-GB -eo -et -fi -fr -gl -ka -de -el -gu -he -hi -hu -is -id -ia -it -ja -kn -kk -km -ky -ko -lt -nds -mk -mai -ms -ml -mr -mn -ne -nb -nn -or -pa -fa -pl -pt -pt-BR -ro -ru -sr -sr@latin -si -sk -sl -es -sv -tg -ta -te -bo -tr -uk -ur -wba -cy -lv -kw@uccor -kw@kkcor -af -am -be -hr -de-CH -th -vi -zu -ilo -nso -tw -yo -anp - - -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer PATCH v2 3/3] Switch to gnulib's compiler warning flags
This enables many more compiler warnings than the current code. It also ensures that -Werror is enabled by default when building from GIT so that maintainers see regressions as hard failures. Reviewed-by: Marc-André Lureau Signed-off-by: Daniel P. Berrangé --- acinclude.m4 | 98 --- configure.ac | 2 +- m4/manywarnings.m4 | 334 + m4/virt-viewer-warnings.m4 | 159 ++ m4/warnings.m4 | 115 + prepare-release.sh | 2 +- 6 files changed, 610 insertions(+), 100 deletions(-) delete mode 100644 acinclude.m4 create mode 100644 m4/manywarnings.m4 create mode 100644 m4/virt-viewer-warnings.m4 create mode 100644 m4/warnings.m4 diff --git a/acinclude.m4 b/acinclude.m4 deleted file mode 100644 index 68398a6..000 --- a/acinclude.m4 +++ /dev/null @@ -1,98 +0,0 @@ -dnl -dnl Taken from gnome-common/macros2/gnome-compiler-flags.m4 -dnl -dnl We've added: -dnl -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls -dnl We've removed -dnl CFLAGS="$realsave_CFLAGS" -dnl to avoid clobbering user-specified CFLAGS -dnl -AC_DEFUN([VIRT_VIEWER_COMPILE_WARNINGS],[ -dnl ** -dnl More compiler warnings -dnl ** - -AC_ARG_ENABLE(compile-warnings, - AC_HELP_STRING([--enable-compile-warnings=@<:@no/minimum/yes/maximum/error@:>@], - [Turn on compiler warnings]),, - [enable_compile_warnings="m4_default([$1],[maximum])"]) - -warnCFLAGS= - -try_compiler_flags="-fexceptions -fstack-protector --param=ssp-buffer-size=4 -fasynchronous-unwind-tables" - -case "$enable_compile_warnings" in -no) - ;; -minimum) - try_compiler_flags="$try_compiler_flags -Wall" - ;; -yes) - try_compiler_flags="$try_compiler_flags -Wall -Wmissing-prototypes -std=c99" - ;; -maximum|error) - try_compiler_flags="$try_compiler_flags -Wall -Wmissing-prototypes -std=c99 -Wnested-externs -Wpointer-arith" -try_compiler_flags="$try_compiler_flags -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return" -# Removed -Wstrict-prototypes to avoid GTK bug - try_compiler_flags="$try_compiler_flags -Winline -Wredundant-decls -Wdeprecated-declarations -Wno-sign-compare" - # Remove as glib function casts hit - try_compiler_flags="$try_compiler_flags -Wno-cast-function-type" - if test "$enable_compile_warnings" = "error" ; then - try_compiler_flags="$try_compiler_flags -Werror" - fi - ;; -*) - AC_MSG_ERROR(Unknown argument '$enable_compile_warnings' to --enable-compile-warnings) - ;; -esac - -AH_VERBATIM([FORTIFY_SOURCE], -[/* Enable compile-time and run-time bounds-checking, and some warnings. */ - #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ - # define _FORTIFY_SOURCE 2 - #endif -]) - -compiler_flags= -for option in $try_compiler_flags; do - SAVE_CFLAGS="$CFLAGS" - CFLAGS="$CFLAGS $option" - AC_MSG_CHECKING([whether gcc understands $option]) - AC_TRY_COMPILE([], [], - has_option=yes, - has_option=no,) - CFLAGS="$SAVE_CFLAGS" - AC_MSG_RESULT($has_option) - if test $has_option = yes; then - compiler_flags="$compiler_flags $option" - fi - unset has_option - unset SAVE_CFLAGS -done -unset option -unset try_compiler_flags - -AC_ARG_ENABLE(iso-c, - AC_HELP_STRING([--enable-iso-c], - [Try to warn if code is not ISO C ]),, - [enable_iso_c=no]) - -AC_MSG_CHECKING(what language compliance flags to pass to the C compiler) -complCFLAGS= -if test "x$enable_iso_c" != "xno"; then - if test "x$GCC" = "xyes"; then - case " $CFLAGS " in - *[\ \ ]-ansi[\ \ ]*) ;; - *) complCFLAGS="$complCFLAGS -ansi" ;; - esac - case " $CFLAGS " in - *[\ \ ]-pedantic[\ \ ]*) ;; - *) complCFLAGS="$complCFLAGS -pedantic" ;; - esac - fi -fi -AC_MSG_RESULT($complCFLAGS) - -WARN_CFLAGS="$compiler_flags $complCFLAGS" -AC_SUBST(WARN_CFLAGS) -]) diff --git a/configure.ac b/configure.ac index 5598c61..8ff69ac 100644 --- a/configure.ac +++ b/configure.ac @@ -90,7 +90,7 @@ m4_if(m4_version_compare([2.61a.100], [AC_CONFIG_LINKS([$GNUmak
[virt-tools-list] [virt-viewer PATCH v2 0/3] Enable many more compiler warnings
This replaces the old GNOME code for compiler warnings with the more advanced GNULIB code, which lets us enable many more warnings. It also ensures that -Werror is enabled for anyone building from GIT to help prevent regressions appearing. Changed in v2: - Pushed already ack'd patches - Use G_GNUC_PRINTF - Fix win32 warnings about strncpy Daniel P. Berrangé (3): Annotate message dialog helpers with format(gnu_printf) Fix warning about missing nul terminator with strncpy Switch to gnulib's compiler warning flags acinclude.m4 | 98 -- configure.ac | 2 +- m4/manywarnings.m4| 334 ++ m4/virt-viewer-warnings.m4| 159 m4/warnings.m4| 115 prepare-release.sh| 2 +- src/virt-viewer-app.c | 38 ++-- src/virt-viewer-notebook.c| 4 +- src/windows-cmdline-wrapper.c | 2 +- 9 files changed, 636 insertions(+), 118 deletions(-) delete mode 100644 acinclude.m4 create mode 100644 m4/manywarnings.m4 create mode 100644 m4/virt-viewer-warnings.m4 create mode 100644 m4/warnings.m4 -- 2.20.1 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer PATCH v2 1/3] Annotate message dialog helpers with format(gnu_printf)
This allows the compiler to validate the format string and args passed
to the function.
Signed-off-by: Daniel P. Berrangé
---
src/virt-viewer-app.c | 38 +++---
src/virt-viewer-notebook.c | 4 ++--
2 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
index 2592484..15e7b3d 100644
--- a/src/virt-viewer-app.c
+++ b/src/virt-viewer-app.c
@@ -197,19 +197,16 @@ virt_viewer_app_set_debug(gboolean debug)
doDebug = debug;
}
-static GtkWidget*
-virt_viewer_app_make_message_dialog(VirtViewerApp *self,
-const char *fmt, ...)
+G_GNUC_PRINTF(2, 0) static GtkWidget*
+virt_viewer_app_make_message_dialogv(VirtViewerApp *self,
+ const char *fmt, va_list vargs)
{
g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), NULL);
GtkWindow *window =
GTK_WINDOW(virt_viewer_window_get_window(self->priv->main_window));
GtkWidget *dialog;
char *msg;
-va_list vargs;
-va_start(vargs, fmt);
msg = g_strdup_vprintf(fmt, vargs);
-va_end(vargs);
dialog = gtk_message_dialog_new(window,
GTK_DIALOG_MODAL |
@@ -224,23 +221,34 @@ virt_viewer_app_make_message_dialog(VirtViewerApp *self,
return dialog;
}
-void
+G_GNUC_PRINTF(2, 3) static GtkWidget*
+virt_viewer_app_make_message_dialog(VirtViewerApp *self,
+const char *fmt, ...)
+{
+g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), NULL);
+GtkWidget *dialog;
+va_list vargs;
+
+va_start(vargs, fmt);
+dialog = virt_viewer_app_make_message_dialogv(self, fmt, vargs);
+va_end(vargs);
+
+return dialog;
+}
+
+G_GNUC_PRINTF(2, 3) void
virt_viewer_app_simple_message_dialog(VirtViewerApp *self,
const char *fmt, ...)
{
GtkWidget *dialog;
-char *msg;
va_list vargs;
va_start(vargs, fmt);
-msg = g_strdup_vprintf(fmt, vargs);
+dialog = virt_viewer_app_make_message_dialogv(self, fmt, vargs);
va_end(vargs);
-dialog = virt_viewer_app_make_message_dialog(self, msg);
gtk_dialog_run(GTK_DIALOG(dialog));
gtk_widget_destroy(dialog);
-
-g_free(msg);
}
static void
@@ -668,7 +676,7 @@ virt_viewer_app_open_unix_sock(const char *unixsock, GError
**error)
#endif /* defined(HAVE_SOCKETPAIR) && defined(HAVE_FORK) */
-void
+G_GNUC_PRINTF(2, 3) void
virt_viewer_app_trace(VirtViewerApp *self,
const char *fmt, ...)
{
@@ -1871,7 +1879,7 @@ virt_viewer_app_on_application_startup(GApplication *app)
if (!virt_viewer_app_start(self, )) {
if (error && !g_error_matches(error, VIRT_VIEWER_ERROR,
VIRT_VIEWER_ERROR_CANCELLED))
-virt_viewer_app_simple_message_dialog(self, error->message);
+virt_viewer_app_simple_message_dialog(self, "%s", error->message);
g_clear_error();
g_application_quit(app);
@@ -2477,7 +2485,7 @@ show_status_cb(gpointer value,
virt_viewer_notebook_show_status(nb, text);
}
-void
+G_GNUC_PRINTF(2, 3) void
virt_viewer_app_show_status(VirtViewerApp *self, const gchar *fmt, ...)
{
va_list args;
diff --git a/src/virt-viewer-notebook.c b/src/virt-viewer-notebook.c
index 3a74e9f..310ea12 100644
--- a/src/virt-viewer-notebook.c
+++ b/src/virt-viewer-notebook.c
@@ -82,7 +82,7 @@ virt_viewer_notebook_init (VirtViewerNotebook *self)
gtk_notebook_append_page(GTK_NOTEBOOK(self), priv->status, NULL);
}
-void
+G_GNUC_PRINTF(2, 0) void
virt_viewer_notebook_show_status_va(VirtViewerNotebook *self, const gchar
*fmt, va_list args)
{
VirtViewerNotebookPrivate *priv;
@@ -99,7 +99,7 @@ virt_viewer_notebook_show_status_va(VirtViewerNotebook *self,
const gchar *fmt,
g_free(text);
}
-void
+G_GNUC_PRINTF(2, 3) void
virt_viewer_notebook_show_status(VirtViewerNotebook *self, const gchar *fmt,
...)
{
va_list args;
--
2.20.1
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer PATCH 01/14] po: provide custom make rules for po file management
Historically we have relied on intltool to install a standard
po/Makefile.in.in which has very limited scope for customization.
intltool is deprecated in favour of standard gettextize tools,
but these share the same disadvantages.
Writing make rules for po file management is no more difficult
than any other rules libvirt-glib has, so stop using intltool
and don't use gettextize ether.
Signed-off-by: Daniel P. Berrangé
---
Makefile.am | 7 --
autogen.sh| 1 -
configure.ac | 6 +-
data/Makefile.am | 12 +-
data/remote-viewer.appdata.xml.in | 12 +-
data/remote-viewer.desktop.in | 4 +-
...iewer-mime.xml.in => virt-viewer-mime.xml} | 0
m4/virt-nls.m4| 45
mingw-virt-viewer.spec.in | 2 +-
po/Makefile.am| 105 ++
po/POTFILES | 26 +
po/POTFILES.in| 27 -
po/README.md | 38 +++
po/lt.po | 2 +-
virt-viewer.spec.in | 2 +-
15 files changed, 234 insertions(+), 55 deletions(-)
rename data/{virt-viewer-mime.xml.in => virt-viewer-mime.xml} (100%)
create mode 100644 m4/virt-nls.m4
create mode 100644 po/Makefile.am
create mode 100644 po/POTFILES
delete mode 100644 po/POTFILES.in
create mode 100644 po/README.md
diff --git a/Makefile.am b/Makefile.am
index d33367a..4cfdd59 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -8,9 +8,6 @@ AM_DISTCHECK_CONFIGURE_FLAGS = --disable-update-mimedb
EXTRA_DIST = \
$(PACKAGE).spec $(PACKAGE).spec.in \
mingw-$(PACKAGE).spec.in\
- intltool-extract.in \
- intltool-merge.in \
- intltool-update.in \
GNUmakefile \
maint.mk\
cfg.mk \
@@ -24,9 +21,6 @@ EXTRA_DIST = \
DISTCLEAN_FILES = \
$(PACKAGE).spec \
mingw-$(PACKAGE).spec \
- intltool-extract\
- intltool-merge \
- intltool-update \
$(NULL)
MAINTAINERCLEANFILES = \
@@ -34,7 +28,6 @@ MAINTAINERCLEANFILES =\
$(srcdir)/INSTALL \
$(GITIGNORE_MAINTAINERCLEANFILES_TOPLEVEL) \
$(GITIGNORE_MAINTAINERCLEANFILES_M4_LIBTOOL)\
- $(srcdir)/m4/intltool.m4\
$(NULL)
dist-hook: gen-ChangeLog gen-AUTHORS
diff --git a/autogen.sh b/autogen.sh
index b77ac33..9c8968f 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -29,7 +29,6 @@ touch ChangeLog AUTHORS
mkdir -p m4
autoreconf -vfi
-intltoolize --force
cd $THEDIR
diff --git a/configure.ac b/configure.ac
index 5598c61..a03d846 100644
--- a/configure.ac
+++ b/configure.ac
@@ -96,8 +96,8 @@ GETTEXT_PACKAGE=virt-viewer
AC_SUBST(GETTEXT_PACKAGE)
AC_DEFINE_UNQUOTED([GETTEXT_PACKAGE],"$GETTEXT_PACKAGE", [GETTEXT package
name])
-AM_GLIB_GNU_GETTEXT
-IT_PROG_INTLTOOL([0.35.0])
+VIRT_VIEWER_ARG_NLS
+VIRT_VIEWER_CHECK_NLS
PKG_PROG_PKG_CONFIG
GLIB_MKENUMS=`$PKG_CONFIG --variable=glib_mkenums glib-2.0`
@@ -261,7 +261,7 @@ AC_CONFIG_FILES([
icons/256x256/Makefile
man/Makefile
mingw-virt-viewer.spec
-po/Makefile.in
+po/Makefile
src/Makefile
src/virt-viewer.rc
tests/Makefile
diff --git a/data/Makefile.am b/data/Makefile.am
index 1f6c8bf..0e50f3d 100644
--- a/data/Makefile.am
+++ b/data/Makefile.am
@@ -68,17 +68,17 @@ else #!WIN32
desktopdir = $(datadir)/applications
DESKTOPFILES = remote-viewer.desktop.in
desktop_DATA = $(DESKTOPFILES:.desktop.in=.desktop)
-@INTLTOOL_DESKTOP_RULE@
+%.desktop: %.desktop.in
+ $(AM_V_GEN)$(MSGFMT) --desktop --template $< -d $(top_srcdir)/po -o $@
-MIMEFILES = virt-viewer-mime.xml.in
mimedir = $(datadir)/mime/packages
mime_DATA = virt-viewer-mime.xml
-@INTLTOOL_XML_RULE@
appdatadir = $(datadir)/appdata
APPDATAFILES = remote-viewer.appdata.xml.in
appdata_DATA = $(APPDATAFILES:.xml.in=.xml)
-@INTLTOOL_XML_RULE@
+%.appdata.xml: %.appdata.xml.in
+ $(AM_V_GEN)$(MSGFMT) --xml --template $< -d $(top_srcdir)/po -o $@
install-data-hook:
if ENABLE_UPDATE_MIMEDB
@@ -92,8 +92,8 @@ if ENABLE_UPDATE_MIMEDB
$(UPDATE_MIME_DATABASE) "$(DESTDIR)$(datadir)/mime";
endif
-CLEANFILES += $(mime_DATA) $(desktop_DATA) $(appdata_DATA)
-EXTRA_DIST += $(MIMEFILES) $(DESKTOPFI
[virt-tools-list] [virt-viewer PATCH 00/14] po: improve translation handling
This applies the same improvements previously done in libvirt: https://www.redhat.com/archives/libvir-list/2018-April/msg01004.html https://www.berrange.com/posts/2018/11/29/improved-translation-po-file-handling-by-ditching-gettext-autotools-integration/ The key problems with our current approach are: - The pot & po files stored in GIT contain huge set of annotations about source file names & line numbers. These are out of date as soon as a change is commited to git following a translation refresh. This makes diffs impossible to meaningfully review, as they are 98% noise, 2% signal. - The po file messages are sorted by source location, so when we move code between files, or rename files, the po file message order changes for no good reason. This makes diffs even more impossible to review. - The po files contain entries for all messages even if most have no translation, bloating size of po/ data stored in git - Whenever 'make dist' is run, it alters all the pot and po files, so developers need to then reset their content to match git HEAD manually. This is caused by having auto-generated content (source file locations) mixed in with the static content (the actual translated strings) After this series, we only minimized po files in git, with the redundated & outdated source locations info stripped. This stripped info is re-added automatically during build to create the real .po files, that we distribute, and/or upload to translators in Zanata. As a result the po directory is smaller in size, and when refreshing from Zanata, we have git commits that clearly show *only* the altered translations, nothing else. The importance of this cannot be overstated - by having these clear diffs when doing this change in libvirt, I discovered a serious bug in the Zanata client that has been screwing up translations in every project that uses Zanata by adding bogus "fuzzy" annotations. The particularly attractive statistic: 193 files changed, 11904 insertions(+), 80334 deletions(-) Daniel P. Berrangé (14): po: provide custom make rules for po file management po: remove language list from zanata configuration po: add rules for integration with zanata po: minimize & canonicalize translations stored in git po: minimize af am anp ar as ast bal be bg po: minimize bn_IN bn bo br brx bs ca cs cy da po: minimize de_CH de el en_GB eo es et eu fa fi po: minimize fr gl gu he hi hr hu ia po: minimize id ilo is it ja ka kk km kn ko po: minimize kw_GB kw@kkcor kw kw@uccor ky lt lv mai mk ml po: minimize mn mr ms nb nds ne nl nn nso po: minimize or pa pl pt_BR pt ro ru si sk sl po: minimize sq sr@latin sr sv ta te tg th tr tw po: minimize uk ur vi wba yo zh_CN zh_HK zh_TW zu Makefile.am |7 - autogen.sh|1 - build-aux/minimize-po.pl | 37 + configure.ac |6 +- data/Makefile.am | 12 +- data/remote-viewer.appdata.xml.in | 12 +- data/remote-viewer.desktop.in |4 +- ...iewer-mime.xml.in => virt-viewer-mime.xml} |0 m4/virt-nls.m4| 45 + mingw-virt-viewer.spec.in |2 +- po/Makefile.am| 115 ++ po/POTFILES | 26 + po/POTFILES.in| 27 - po/README.md | 76 ++ po/af.mini.po | 19 + po/af.po | 864 - po/am.mini.po | 19 + po/am.po | 864 - po/anp.mini.po| 19 + po/anp.po | 864 - po/ar.mini.po | 20 + po/ar.po | 869 - po/as.mini.po | 327 + po/as.po | 884 - po/ast.mini.po| 19 + po/ast.po | 864 - po/bal.mini.po| 19 + po/bal.po | 863 - po/be.mini.po | 20 + po/be.po | 866 - po/bg.mini.po | 307 + po/bg.po | 881 - po/bn.mini.po | 256 po/bn.po | 885 - po/bn_IN.mini.po | 329 + po/bn_IN.po | 886
Re: [virt-tools-list] [virt-viewer PATCH] Add a git-publish configuration file
On Tue, Feb 19, 2019 at 04:05:00PM +0100, Marc-André Lureau wrote: > Hi > On Tue, Feb 19, 2019 at 4:03 PM Daniel P. Berrangé > wrote: > > > > Signed-off-by: Daniel P. Berrangé > > --- > > .gitpublish | 4 > > 1 file changed, 4 insertions(+) > > create mode 100644 .gitpublish > > > > diff --git a/.gitpublish b/.gitpublish > > new file mode 100644 > > index 000..bf82571 > > --- /dev/null > > +++ b/.gitpublish > > @@ -0,0 +1,4 @@ > > +[gitpublishprofile "default"] > > +base = master > > +to = virt-tools-list@redhat.com > > +prefix = virt-viewer PATCH > > In general, I believe "PATCH virt-viewer" order is more common. When we did this for libvirt, the exact opposite was asserted, hence we used "$module PATCH". I think it does make sense to have the "PATCH" word next to the patch sequence numbers too. I'd like to be consistent with libvirt in this area in any case. 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 :| ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
