Hi @Andrea,
Is there any chance that these patches catch up with 4.7.0 release?
I see it's entering freeze.
Expecting your comments.
Thanks very much!
Yi Min
在 2018/8/27 下午1:48, Yi Min Zhao 写道:
Abstract
The PCI representation in QEMU has recently been extended for S390
allowing configuration of zPCI attributes like uid (user-defined
identifier) and fid (PCI function identifier).
The details can be found here:
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html
To support the new zPCI feature of the S390 platform, two new XML
attributes, @uid and @fid, are introduced for device addresses of type
'pci', i.e.:
uid and fid are optional attributes. If they are defined by the user,
unique values within the guest domain must be used. If they are not
specified and the architecture requires them, they are automatically
generated with non-conflicting values.
Current implementation is the most seamless one for the user as it
unites the address specific data of a PCI device on one XML element.
It could accommodate both specifying our special parameters (uid and fid)
and re-using standard statements (domain, bus, slot and function) for
PCI devices. User can still specify bus/slot/function for the virtualized
PCI devices in the XML.
Thus uid/fid act as an extension to the PCI address and are stored in
a new structure 'virZPCIDeviceAddress' which is a member of common PCI
Address structure. Additionally, two hashtables are used for assignment
and reservation of uid/fid.
In support of extending the PCI address, a new PCI address extension flag is
introduced. This extension flag allows is not only dedicated for the S390
platform but also other architectures needing certain extensions to PCI
address space.
Code Base
=
commit in master:
3b89e1f962 docs: api_extension: Don't encourage other tools than git
Change Log
==
v3->v4:
1. Update docs.
2. Format code style.
3. Optimize zPCI support check.
4. Move the check of zPCI defined in xml but unsupported by Qemu to
qemuDomainDeviceDefValidate().
5. Change zpci address member of PCI address struct from pointer to
instance.
6. Modify zpci address definition principle. Currently the user must
either define both of uid and fid or not.
v2->v3:
1. Revise code style.
2. Update test cases.
3. Introduce qemuDomainCollectPCIAddressExtension() to collect PCI
extension addresses.
4. Introduce virDeviceInfoPCIAddressExtensionPresent() to check if zPCI
address exists.
5. Optimize zPCI address check logic.
6. Optimize passed parameters of zPCI addr alloc/release/reserve functions.
7. Report enum range error in qemuDomainDeviceSupportZPCI().
8. Update commit messages.
v1->v2:
1. Separate test commit and merge testcases into corresponding commits that
introduce the functionalities firstly.
2. Spare some checks for zpci device.
3. Add vsock and controller support.
4. Add uin32 type schema.
5. Rename zpciuid and zpcifid to zpci_uid and zpci_fid.
6. Always return multibus support on S390.
Yi Min Zhao (12):
conf: Add definitions for 'uid' and 'fid' PCI address attributes
qemu: Introduce zPCI capability
conf: Introduce a new PCI address extension flag
qemu: Enable PCI multi bus for S390 guests
qemu: Auto add pci-root for s390/s390x guests
conf: Introduce address caching for PCI extensions
conf: Introduce parser, formatter for uid and fid
conf: Allocate/release 'uid' and 'fid' in PCI address
qemu: Generate and use zPCI device in QEMU command line
qemu: Add hotpluging support for PCI devices on S390 guests
docs: Add 'uid' and 'fid' information
news: Update news for PCI address extension attributes
cfg.mk | 1 +
docs/formatdomain.html.in | 9 +-
docs/news.xml | 10 +
docs/schemas/basictypes.rng| 23 ++
docs/schemas/domaincommon.rng | 1 +
src/conf/device_conf.c | 62
src/conf/device_conf.h | 14 +
src/conf/domain_addr.c | 331 +
src/conf/domain_addr.h | 29 ++
src/conf/domain_conf.c | 6 +
src/libvirt_private.syms | 4 +
src/qemu/qemu_capabilities.c | 6 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c| 95 ++
src/qemu/qemu_command.h| 2 +
src/qemu/qemu_domain.c | 27 ++
src/qemu/qemu_domain_address.c | 205 -
src/qemu/qemu_hotplug.c| 151 +-
src/util/virpci.h | 11 +
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1
Michal Prívozník [2018-08-27, 04:03PM +0200]:
> On 08/27/2018 03:20 PM, Marc Hartmayer wrote:
> > If @vm has flagged as "to be removed" virDomainObjListFindByNameLocked
> > returns NULL (although the definition actually exists). Therefore, the
> > possibility exits that "virHashAddEntry" will raise the error
> > "Duplicate key" => virDomainObjListAddObjLocked fails =>
> > virDomainObjEndAPI(&vm) is called and this leads to a freeing of @def
> > since @def is already assigned to vm->def. But actually this leads to
> > a double free since the common usage pattern is that the caller of
> > virDomainObjListAdd(Locked) is responsible for freeing @def in case of
> > an error.
> >
> > Let's fix this by setting vm->def to NULL in case of an error.
> >
> > Backtrace:
> >
> >➤ bt
> >#0 virFree (ptrptr=0x7575757575757575)
> >#1 0x03ffb5b25b3e in virDomainResourceDefFree
> >#2 0x03ffb5b37c34 in virDomainDefFree
> >#3 0x03ff9123f734 in qemuDomainDefineXMLFlags
> >#4 0x03ff9123f7f4 in qemuDomainDefineXML
> >#5 0x03ffb5cd2c84 in virDomainDefineXML
> >#6 0x00011745aa82 in remoteDispatchDomainDefineXML
> >...
> >
> > Reviewed-by: Bjoern Walk
> > Signed-off-by: Marc Hartmayer
> > ---
> > src/conf/virdomainobjlist.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
> > index 52171594f34f..805fe9440afe 100644
> > --- a/src/conf/virdomainobjlist.c
> > +++ b/src/conf/virdomainobjlist.c
> > @@ -329,8 +329,10 @@ virDomainObjListAddLocked(virDomainObjListPtr doms,
> > goto cleanup;
> > vm->def = def;
> >
> > -if (virDomainObjListAddObjLocked(doms, vm) < 0)
> > +if (virDomainObjListAddObjLocked(doms, vm) < 0) {
> > +vm->def = NULL;
> > goto error;
> > +}
> > }
> > cleanup:
> > return vm;
> >
>
>
> How about setting vm->def only after virDomainObjListAddObjLocked()
> succeded?
>
> However, this points to a more sever bug. If a domain is being removed,
> and some other thread is trying to define it back, I guess the whole
> operation should succeed. Definitely not fail with some (for user)
> cryptic message like "double key found in a hash table".
>
> The problem is that:
>
> a) both virDomainObjListFindByUUIDLocked() and
> virDomainObjListFindByNameLocked() return NULL even if domain exists.
> They should return the found domain and only the caller should decide if
> vm->removing is relevant or not.
>
> b) nothing is clearing out the vm->removing flag. The
> virDomainObjListAddObjLocked() looks like a good candidate to do so.
>
> Michal
Can we still take this fix for the upcoming release and worry about
doing it right later?
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
IBM Systems
Linux on Z & Virtualization Development
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/23/2018 03:27 AM, Han Han wrote:
> Fix logical error in v2:
> https://www.redhat.com/archives/libvir-list/2018-August/msg01358.html
>
^^^ This would go under the --- below so it's not part of the history.
> https://bugzilla.redhat.com/show_bug.cgi?id=1619625
>
> --all option is added to cmdDomblkinfo since commit 62c39193 allowing to
> show all block devices info. Reset error when empty source in case error
> breaks the loop of domblkinfo for disks.
>
> Signed-off-by: Han Han
> ---
> tools/virsh-domain-monitor.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index b9b4f9739b..ecb98d4128 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -475,6 +475,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
> int ndisks;
> size_t i;
> xmlNodePtr *disks = NULL;
> +char *source = NULL;
> char *target = NULL;
> char *protocol = NULL;
>
> @@ -505,18 +506,20 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>
> for (i = 0; i < ndisks; i++) {
> ctxt->node = disks[i];
> +source = virXPathString("string(./source)", ctxt);
> protocol = virXPathString("string(./source/@protocol)", ctxt);
> target = virXPathString("string(./target/@dev)", ctxt);
I *think* what Peter was inferring from his v1 review is that if
!source, then don't make that virDomainGetBlockInfo call since it's just
going to fail since an empty cdrom will have no . Thus avoiding
the error from qemuDomainGetBlockInfo:
if (virStorageSourceIsEmpty(disk->src)) {
virReportError(VIR_ERR_INVALID_ARG,
_("disk '%s' does not currently have a source
assigned"),
path);
goto endjob;
}
How about:
/* Skip GetBlockInfo if no , it's an indication of an
* empty CDROM device. */
rc = 0;
if (source)
rc = virDomainGetBlockInfo(dom, target, &info, 0);
else
memset(&info, 0, sizeof(info));
This should be enough to satisfy the bz issue and not change other code
as well.
>
> rc = virDomainGetBlockInfo(dom, target, &info, 0);
>
> if (rc < 0) {
> -/* If protocol is present that's an indication of a networked
> - * storage device which cannot provide statistics, so
> generate
> - * 0 based data and get the next disk. */
> -if (protocol && !active &&
> +/* For the case of empty cdrom, networked disk which cannot
> + * provide statistics, generate 0 based data and get the next
> + * disk.
> + */
> +if (!source || (protocol && !active &&
Don't change anything here. When you have a protocol and it's !active
and you have rc < 0, we more than likely have that specific existing
error scenario which would come from qemuStorageLimitsRefresh failure
through virStorageFileInitAs.
John
> virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR &&
> -virGetLastErrorDomain() == VIR_FROM_STORAGE) {
> +virGetLastErrorDomain() == VIR_FROM_STORAGE)) {
> memset(&info, 0, sizeof(info));
> vshResetLibvirtError();
> } else {
> @@ -526,6 +529,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>
> cmdDomblkinfoPrint(ctl, &info, target, human, false);
>
> +VIR_FREE(source);
> VIR_FREE(target);
> VIR_FREE(protocol);
> }
> @@ -540,6 +544,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>
> cleanup:
> virshDomainFree(dom);
> +VIR_FREE(source);
> VIR_FREE(target);
> VIR_FREE(protocol);
> VIR_FREE(disks);
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
From: Marc-André Lureau
Similar to the qemu_tpm.c, add a unit with a few functions to
start/stop and setup the cgroup of the external vhost-user-gpu
process. See function documentation.
Signed-off-by: Marc-André Lureau
---
src/conf/device_conf.h | 1 +
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_vhost_user_gpu.c | 318 +
src/qemu/qemu_vhost_user_gpu.h | 48 +
4 files changed, 369 insertions(+)
create mode 100644 src/qemu/qemu_vhost_user_gpu.c
create mode 100644 src/qemu/qemu_vhost_user_gpu.h
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index ff7d6c9d5f..79a7ea9fe2 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -175,6 +175,7 @@ struct _virDomainDeviceInfo {
* cases we might want to prevent that from happening by
* locking the isolation group */
bool isolationGroupLocked;
+int vhost_user_fd;
};
int virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst,
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 2afa67f195..28daf9d426 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -56,6 +56,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_qapi.h \
qemu/qemu_tpm.c \
qemu/qemu_tpm.h \
+ qemu/qemu_vhost_user_gpu.c \
+ qemu/qemu_vhost_user_gpu.h \
$(NULL)
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
new file mode 100644
index 00..9007614020
--- /dev/null
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -0,0 +1,318 @@
+/*
+ * qemu_vhost_user_gpu.c: QEMU vhost-user GPU support
+ *
+ * Copyright (C) 2018 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * .
+ *
+ * Author: Marc-André Lureau
+ */
+
+#include
+
+#include "qemu_extdevice.h"
+#include "qemu_domain.h"
+#include "qemu_security.h"
+
+#include "conf/domain_conf.h"
+#include "vircommand.h"
+#include "viralloc.h"
+#include "virlog.h"
+#include "virutil.h"
+#include "virfile.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virpidfile.h"
+#include "qemu_vhost_user_gpu.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("qemu.vhost-user-gpu")
+
+/*
+ * Look up the vhost-user-gpu executable; to be found on the host
+ */
+static char *vhost_user_gpu_path;
+
+static int
+qemuVhostUserGPUInit(void)
+{
+if (!vhost_user_gpu_path) {
+vhost_user_gpu_path = virFindFileInPath("vhost-user-gpu");
+if (!vhost_user_gpu_path) {
+virReportSystemError(ENOENT, "%s",
+ _("Unable to find 'vhost-user-gpu' binary in
$PATH"));
+return -1;
+}
+if (!virFileIsExecutable(vhost_user_gpu_path)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("vhost-user-gpu %s is not an executable"),
+ vhost_user_gpu_path);
+VIR_FREE(vhost_user_gpu_path);
+return -1;
+}
+}
+
+return 0;
+}
+
+
+static char *
+qemuVhostUserGPUCreatePidFilename(const char *stateDir,
+ const char *shortName,
+ const char *alias)
+{
+char *pidfile = NULL;
+char *devicename = NULL;
+
+if (virAsprintf(&devicename, "%s-%s-vhost-user-gpu", shortName, alias) < 0)
+return NULL;
+
+pidfile = virPidFileBuildPath(stateDir, devicename);
+
+VIR_FREE(devicename);
+
+return pidfile;
+}
+
+
+/*
+ * qemuVhostUserGPUGetPid
+ *
+ * @stateDir: the directory where vhost-user-gpu writes the pidfile into
+ * @shortName: short name of the domain
+ * @alias: video device alias
+ * @pid: pointer to pid
+ *
+ * Return -errno upon error, or zero on successful reading of the pidfile.
+ * If the PID was not still alive, zero will be returned, and @pid will be
+ * set to -1;
+ */
+static int
+qemuVhostUserGPUGetPid(const char *stateDir,
+ const char *shortName,
+ const char *alias,
+ pid_t *pid)
+{
+int ret;
+char *pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName,
alias);
+if (!pidfile)
+return -ENOMEM;
+
+ret = virPidFileReadPathIfAlive(pidfile, pid, vhost_user_gpu_path);
+
+VIR_FREE(pidfile);
+
+return ret;
+}
+
+
+/*
+ * qemuExtVhostUserGPUStart:
+ *
+ * @driver: QEMU driver
From: Marc-André Lureau
Hi,
This series of patches add support for running a virtio GPU in a
seperate process, using vhost-user.
The QEMU series "[PATCH v4 00/29] vhost-user for input & GPU" is still
under review, and will hopefully land in 3.1. There are several
benefits of running the GPU process in an external process, since Mesa
is rather heavy on the qemu main loop, and may block for a while or
crash. I observe x5 performance improvements with Unigine Heaven 4
benchmark.
The external GPU process is started with one end of the vhost-user
socket pair, the other end is given to a QEMU chardev. It is also
added to the emulator cgroup to restrict its CPU usage.
vhost-user requires shared VM memory. The first patches ease and
improve shared memory setup, by using memfd. They could be considered
seperatly, but that's the setup I'd recommend with vhost-user-gpu.
Review welcome!
RFCv2:
- add new memfd memroyBacking source type
- drop the implicit shared memory NUMA setup approach, explicit now
required
- rebased
Marc-André Lureau (16):
qemu: add memory-backend-memfd capability check
qemu: add memfd memory backing
qemu: add vhost-user-gpu capabilities checks
domain: add "vhost-user" video type
qemu: fill the vhost-user video type capability
qemu: check that qemu is vhost-user-vga capable
qemu: vhost-user is valid as non-primary video device
qemu: validate vhost-user video model
qemu: add qemuSecurityStartVhostUserGPU helper
qemu: add vhost-user-gpu helper unit
qemu: restrict 'virgl=' option to 'virtio' video type
qemu: set default address type on vhost-user video model
qemu: start/stop the vhost-user-gpu external device
qemu: build vhost-user-backend for vhost-user-gpu
qemu: build vhost-user-gpu video device arguments
tests: add vhost-user-gpu xml2argv tests
docs/formatdomain.html.in | 11 +-
docs/schemas/domaincommon.rng | 2 +
src/conf/device_conf.h| 1 +
src/conf/domain_conf.c| 7 +-
src/conf/domain_conf.h| 2 +
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_capabilities.c | 8 +
src/qemu/qemu_capabilities.h | 3 +
src/qemu/qemu_command.c | 135 ++--
src/qemu/qemu_domain.c| 8 +-
src/qemu/qemu_domain_address.c| 4 +-
src/qemu/qemu_extdevice.c | 47 ++-
src/qemu/qemu_process.c | 6 +-
src/qemu/qemu_security.c | 48 +++
src/qemu/qemu_security.h | 6 +
src/qemu/qemu_vhost_user_gpu.c| 318 ++
src/qemu/qemu_vhost_user_gpu.h| 48 +++
tests/domaincapsschemadata/full.xml | 1 +
.../caps_2.12.0.aarch64.xml | 1 +
.../caps_2.12.0.ppc64.xml | 1 +
.../caps_2.12.0.s390x.xml | 1 +
.../caps_2.12.0.x86_64.xml| 1 +
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 +
.../caps_3.0.0.riscv32.xml| 1 +
.../caps_3.0.0.riscv64.xml| 1 +
.../caps_3.0.0.x86_64.xml | 1 +
tests/qemuxml2argvdata/memfd-memory-numa.args | 27 ++
tests/qemuxml2argvdata/memfd-memory-numa.xml | 33 ++
.../vhost-user-gpu-secondary.args | 33 ++
.../vhost-user-gpu-secondary.xml | 44 +++
tests/qemuxml2argvdata/vhost-user-vga.args| 30 ++
tests/qemuxml2argvdata/vhost-user-vga.xml | 41 +++
tests/qemuxml2argvtest.c | 15 +
33 files changed, 849 insertions(+), 39 deletions(-)
create mode 100644 src/qemu/qemu_vhost_user_gpu.c
create mode 100644 src/qemu/qemu_vhost_user_gpu.h
create mode 100644 tests/qemuxml2argvdata/memfd-memory-numa.args
create mode 100644 tests/qemuxml2argvdata/memfd-memory-numa.xml
create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.args
create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml
create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.args
create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.xml
--
2.19.0.rc0.48.gb9dfa238d5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> As advertised in previous commit, there are three APIs that might
> run for quite some time (because they read/write data from/to a
> volume) and these three are: downloadVol, uploadVol, wipeVol.
> Release pool object lock and reacquire it later to allow more
> concurrency.
>
> Signed-off-by: Michal Privoznik
> ---
> src/storage/storage_backend_iscsi_direct.c | 6 +-
> src/storage/storage_backend_rbd.c | 8 ++--
> src/storage/storage_driver.c | 6 ++
> 3 files changed, 17 insertions(+), 3 deletions(-)
>
I think we could/should note in src/storage/storage_backend.h that for
the three functions (Download, Upload, and Wipe) that upon entry the
@obj for the pool is unlocked, but the pool object's "asyncjobs" will be
adjusted to allow concurrency and that the the volume's "in_use"
adjusted to ensure singular usage.
I'd also add a similar comment to virStorageBackendVolWipeLocal,
virStorageBackendDiskVolWipe, virStorageBackendVolUploadLocal, and
virStorageBackendVoldownloadLocal that the @pool is unlocked prior to
the call.
If virStorageBackendRBDVolWipe or virStorageBackenISCSIDirectWipeVol get
a similar information is up to you, I'd probably put it there too just
for completeness for the "next" poor soul that copies from one of the
existing backends.
Whether or not anyone actually reads it is their problem, at least we
noted it!
Reviewed-by: John Ferlan
John
FWIW: I almost had an oh sh*t moment with virObject{Unlock|Lock} before
I came back in off the ledge since we're not talking the pools' rwlock,
but the pool's obj/def lock that isn't an rwlock.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> There are few operations in the storage driver that read/write
> data onto volumes. Such operations can take very long time to
> finish. During that time the storage pool object is locked which
> has bad performance impacts (other threads can't fetch its XML
> for instance). This commit prepares the storage driver for
> releasing the lock during those operations (downloadVol,
> uploadVol, wipeVol).
The performance concurrency implications description could just be moved
to the next patch instead of indicating there "As advertised".
For this commit, noting that we are preparing for future release of the
pool lock while one of the upload, download, or wipe volumes operations
is occurring. This means adjusting the asyncJob count to inhibit pool
operations that would be affected by the volume change from occurring
while we're in the process of upload, download, or wipe of a volume and
similarly adjusting the volume in_use count to inhibit other competing
operations from occurring (or something really generic).
>
> Signed-off-by: Michal Privoznik
> ---
> src/storage/storage_driver.c | 34 --
> 1 file changed, 28 insertions(+), 6 deletions(-)
>
Reviewed-by: John Ferlan
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Aug 15, 2018 at 06:35:47AM -0300, Marcos Paulo de Souza wrote:
> Hi guys,
>
> this is the second version of the patch, the first one can be found here[1].
> This version addresses the comments from Matthias Bolte, making the change
> simpler and cleaner.
>
> Let me know if there are other details that needs to change.
>
> [1]: https://www.redhat.com/archives/libvir-list/2018-August/msg00532.html
>
> Marcos Paulo de Souza (1):
> esx: Make esxDomainGetVcpusFlags check flags
>
> src/esx/esx_driver.c | 52
> 1 file changed, 28 insertions(+), 24 deletions(-)
>
> --
> 2.17.1
>
Checked now that libvirt is entering in "freezer", so you guys please take a
look at this patch?
--
Thanks,
Marcos
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Sorry I forgot to announce it but we are the last week of the month,
so it's about time, I tagged RC1 in git and pushed the signed tarball
and rpms to the usual place earlier today:
ftp://libvirt.org/libvirt/
this seems to work well in my limited testing but as usual we need more
review and check on other platforms or OSes before the release. In general
https://ci.centos.org/view/libvirt/ is green except for glib bindings and
for the master build (weird!)
I will likely push the rc2 Thursday morning, but I mwill be travelling at
the end of the week so that may depends on conditions, and hopefully
the release during this w.e.
thanks for giving it a try !
Daniel
--
Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> Just like a few commits earlier, checking for pool source
> duplicates and unlocking pools list afterwards is a buggy
> pattern. The check must go into virStoragePoolObjAssignDef.
Probably should be noted that this also swaps the order of checking from
pool name/uuid duplication, then pool source duplication to pool source,
then pool name/uuid. In the long run it doesn't matter and this new
order is fine. I assume the current order was chosen because it's
quicker to HashLookup as opposed to HashSearch (although lately making
assumptions hasn't necessarily worked out for me).
Another change is now the duplication checks are held under a write lock
instead of a concurrent read lock.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/virstorageobj.c | 7 ---
> src/conf/virstorageobj.h | 4
> src/libvirt_private.syms | 1 -
> src/storage/storage_driver.c | 6 --
> 4 files changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index dce45ce870..c717176133 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -1486,17 +1486,15 @@ virStoragePoolObjSourceFindDuplicateCb(const void
> *payload,
> }
>
>
> -int
> +static int
> virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools,
> virStoragePoolDefPtr def)
Let's take the opportunity to rename this too
virStoragePoolObjAssignIsDuplicatePoolSource
Return true/false too... All can be done in the same patch.
Reviewed-by: John Ferlan
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> The @conn argument is needed only to do some source matching in
> case of iSCSI source. Anyway, it's used just for node device
> driver and as such can be replaced with virGetConnectNodeDev().
NB: Since commit 2870419eb enabled opening connections to secondary
drivers at time of use, we no longer have to carry around the storage
driver connection in order to use for the connection.
(or something close to what commit decaeb28 describes)
>
> At the same time, the @conn struct member is dropped from
> _virStoragePoolObjFindDuplicateData.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/virstorageobj.c | 25 +++--
> src/conf/virstorageobj.h | 3 +--
> src/storage/storage_driver.c | 4 ++--
> 3 files changed, 14 insertions(+), 18 deletions(-)
>
Reviewed-by: John Ferlan
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/28/2018 09:58 AM, Michal Privoznik wrote:
> On 08/28/2018 03:30 PM, John Ferlan wrote:
>>
>>
>> On 08/20/2018 08:09 AM, Michal Privoznik wrote:
>>> In two places the passed pool object argument is marked as
>>> ATTRIBUTE_UNUSED even though it's used right away.
>>>
>>> Signed-off-by: Michal Privoznik
>>> ---
>>> src/storage/storage_backend_rbd.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> Reviewed-by: John Ferlan
>>
>> Although freeze hasn't been announced, an rc1 tag exists - so I'll state
>> that although the first 2 patches could be designated safe for freeze,
>> the rest of the series needs to wait until after 4.7.0.
>
> Well, the whole patch set can be viewed as a bug fix and as such is
> exempt from freeze ;-)
>
Careful there you don't want to set a dangerous precedent, that I could
use ;-)... Still, I see no bz associated with any patch in the series.
Also there are three distinct things happening in this series.
1. Moving the Is/Find duplicate code into AssignDef for TOCTOU. That's a
lot of moving parts and some amount of logic adjustment that should get
more exposure than a few RC days.
2. The usage of in_use during download, upload, and wipe
3. Release of pool lock while in_use is set for more concurrency
I think 2 is a "easy" bug, 1 is a "hard" bug, and 3 is less a bug and
more a concurrency enhancement.
When it comes to late in the game, trivial things (like patch 1), easy
bugs, patches associated w/ customer bzs, and crash/core type bugs
could/should be fixed. But harder to reproduce and/or present in more
than the current (months) release can/should wait.
Of course JMO...
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> This function is going to be made static in used in
> virStoragePoolObjAssignDef(). Therefore move it and all the
> static functions it calls a few lines up.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/virstorageobj.c | 788
> +++
> 1 file changed, 394 insertions(+), 394 deletions(-)
>
Ugh, what a mess git diff makes... need some --patience on the diff
Still consider my patch 3 suggestion about combining this with patch
2... It then just becomes a "reorganize the code" type patch where all
the Add/Del, GetNum, GetName, and Export are lumped together.
Either way,
Reviewed-by: John Ferlan
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> Even though we do some checking is not as thorough as it should
s/is not/it is not/
> be. We already have virStoragePoolObjIsDuplicate but the way we
> use it is a typical TOCTOU. Imagine two threads trying to define
> two pools with the same name but different UUIDs. With the
> current code neither of them finds a duplicate and thus proceed
> to virStoragePoolObjAssignDef where only names are compared.
> Therefore both threads succeed which is obviously wrong.
>
> We should check for duplicates where we care for them.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/virstorageobj.c | 39 +++
> src/conf/virstorageobj.h | 8 ++--
> src/libvirt_private.syms | 1 -
> src/storage/storage_driver.c | 10 ++
> src/test/test_driver.c | 12 +++-
> 5 files changed, 34 insertions(+), 36 deletions(-)
>
After seeing patch 4, would it just be worthwhile to merge it into patch
2. There's 405 lines from the virStoragePoolObjAssignDef comments to
that could move to just above virStoragePoolObjMatch. IDC either way,
but since the patches that follow work off it it...
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index 185822451b..ea0ae6fd86 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -1052,22 +1052,28 @@ virStoragePoolObjVolumeListExport(virConnectPtr conn,
> * @doms : virStoragePoolObjListPtr to search
> * @def : virStoragePoolDefPtr definition of pool to lookup
> * @check_active: If true, ensure that pool is not active
> + * @objRet: returned pool object
> *
> - * Returns: -1 on error
> + * Assumes @pools is locked by caller already.
> + *
> + * Returns: -1 on error (name or UUID mismatch)
-1 on error (name/uuid mismatch *or* check_active failure)
and /me wonders if we should move the check_active failure to the caller
> * 0 if pool is new
> - * 1 if pool is a duplicate
> + * 1 if pool is a duplicate (name and UUID match)
"Implied" -1 and 0 return means obj == NULL, while 1 means obj is set.
> */
> -int
> +static int
> virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
I think we should change the name to be
virStoragePoolObjAssignFindPool
or something similar since "IsDuplicate" probably should have returned
true/false only given other libvirt method naming schemes. I'd be OK
doing it all in one change too.
Reviewed-by: John Ferlan
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2018-08-28 at 17:49 +0200, Ján Tomko wrote:
> We should consider auto-adding the device to domain XML, if it's always
> present in the machine type.
I'll look into it.
I wonder how many other architectures are affected by the
same issue - at the very least ARM, I think.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 05:56:25PM +0200, Andrea Bolognani wrote:
We would have used virtio for networking anyway, but it's
better to be explicit; for graphics, none of the existing
models work right now but virtio is the only one which
has a non-PCI variant, so it's as good a default as any.
Spotted-by: Ján Tomko
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_domain.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
We would have used virtio for networking anyway, but it's
better to be explicit; for graphics, none of the existing
models work right now but virtio is the only one which
has a non-PCI variant, so it's as good a default as any.
Spotted-by: Ján Tomko
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_domain.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 22c6d8f090..2410add9c2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5840,6 +5840,10 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
return "lan9118";
}
+/* virtio is a sensible default for RISC-V virt guests */
+if (qemuDomainIsRISCVVirt(def))
+return "virtio";
+
/* In all other cases the model depends on the capabilities. If they were
* not provided don't report any default. */
if (!qemuCaps)
@@ -6326,7 +6330,9 @@ qemuDomainDeviceVideoDefPostParse(virDomainVideoDefPtr
video,
if (video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) {
if (ARCH_IS_PPC64(def->os.arch))
video->type = VIR_DOMAIN_VIDEO_TYPE_VGA;
-else if (qemuDomainIsARMVirt(def) || ARCH_IS_S390(def->os.arch))
+else if (qemuDomainIsARMVirt(def) ||
+ qemuDomainIsRISCVVirt(def) ||
+ ARCH_IS_S390(def->os.arch))
video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
else
video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Aug 27, 2018 at 02:10:18PM +0200, Andrea Bolognani wrote:
None of the existing models is suitable for use with
RISC-V virt guests, and we don't want information about
the serial console to be missing from the XML.
The name is based on comments in qemu/hw/riscv/virt.c:
RISC-V machine with 16550a UART and VirtIO MMIO
and in qemu/hw/char/serial.c:
QEMU 16550A UART emulation
along with the output of dmesg in the guest:
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
1000.uart: ttyS0 at MMIO 0x1000 (irq = 13,
base_baud= 230400) is a 16550A
Signed-off-by: Andrea Bolognani
---
CC'ing Rich mostly so that he can double-check the name
choice is sensible.
docs/formatdomain.html.in | 12 +-
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c| 1 +
src/conf/domain_conf.h| 1 +
src/qemu/qemu_command.c | 12 ++
src/qemu/qemu_domain.c| 27 ---
tests/qemuxml2xmloutdata/riscv64-virt.xml | 4 +++-
7 files changed, 44 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko
We should consider auto-adding the device to domain XML, if it's always
present in the machine type.
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Aug 24, 2018 at 02:03:22PM +0200, Andrea Bolognani wrote:
The architecture is new enough that we don't need to
concern ourselves with backwards compatibility in any
capacity.
Signed-off-by: Andrea Bolognani
---
Requires
https://www.redhat.com/archives/libvir-list/2018-June/msg01223.html
to be applied (and test data to be refreshed in the process).
With the above conditions satisfied:
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Aug 27, 2018 at 06:13:57PM +0200, Peter Krempa wrote:
Peter Krempa (3):
tests: qemumonitorjson: Add test for 'block-stream' command
tests: qemumonitorjson: Change values which would be omitted
tests: qemumonitorjson: Add test case for 'blockdev-mirror'
tests/qemumonitorjsontest.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:43:29PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa
---
v2:
- switched from '$SECTION' to '$MODULE' as a hint for the split files
- changed 'domain.rng' to 'domaincommon.rng' which contains the defs
- added the '$MODULE' to virsh path
Note some still may be wrong as I did not chec all paths rigorously.
docs/api_extension.html.in | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/28/2018 10:11 AM, Daniel P. Berrangé wrote:
> On Tue, Aug 28, 2018 at 04:10:53PM +0200, Erik Skultety wrote:
>> On Tue, Aug 28, 2018 at 03:52:56PM +0200, Ján Tomko wrote:
>>> On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
>> On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
>>> On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> So how about storing 2 sets of expected data for this test case.
>
>>>
>>> Two is not enough. My clang 5.0.1 produces a test that displays the
>>> monkeys correctly, but does not count their width properly:
>>
>> Is this a different bug though ? The issue with iswprint() is varying
>> according to glibc version, not compiler version.
>>
>
> The broken setup is:
> sys-libs/glibc-2.25-r9
> sys-devel/clang-5.0.1
>
> It works on:
> sys-libs/glibc-2.26-r7
> with either of:
> sys-devel/clang-5.0.1
> sys-devel/clang-6.0.1
>
> So yes, it is a glibc bug.
> Depending on the version, either just wcwidth returns incorrect values
> for the monkeys (my case) or iswprint considers them non-printable.
It sounds like in your case we're genuinely broken in the virsh
code, not merely tests broken.
I wonder if we need extra logic in the virsh code to handle escaping
for the cases where wcwidth is returning wrong data, so we still get
column layout correct ?
>>>
>>> I don't think so.
>>>
>>> 1) wouldn't that involve reimplementing the wcwidth function?
>>> 2) users crazy enough to use new unicode characters are welcome to
>>> upgrade to new glibc, or suffer through misaligned virsh tables.
>>
>> I second that, I don't think that in this case it's any beneficial trying to
>> fix glibc problems just to offer users a consistent experience, we're not
>> gnulib, besides, we're talking about table alignment which has been broken
>> for ages and we've already made a significant progress here. Additionally, as
>> Jano has pointed out, if someone feels adventurous, that's fine, but it
>> may come with certain limitations.
>
> Ok, so we just need get the test to correctly skip on platforms where
> we know it won't get right answers
>
>
Cannot believe the effort/time on this...
The problem with bypassing on such platforms is how does one then know
those platforms get fixed so that the "skip" can be removed? Of course
seeing test failure is not good either.
There is no simple answer, we can try to code around this or we can
accept on certain platforms the specific test may fail for a specific
reason and move on.
In the long run, it's a format that what percentage of our users will
use? Is naming domains or "other" libvirt objects using emojis a real
world problem that we really care to ensure that the table lines up
properly? Maybe it's just best to determine that the name has one of
those and accept that calculations are going to be wrong. Maybe we just
decide if we see one in a name (e.g. a non normally printable or greater
than some charset value via 0x) that we "assume" or "assign" the
maximum width possible for each of those characters found and move on.
Maybe we should "Validate" that a name doesn't have those chars and if
it does fail to recognize using a new errno code we invent, "ENO🙊🙉🙈,
usage of emoticons as names or keys is not supportable".
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
When switching the host architecture to something for which we do not
have any host CPU model defined, the mocked
virQEMUCapsProbeHostCPUForEmulator would just return the previous CPU
model resulting in strange combinations, such as "core2duo" host CPU
model in QEMU capabilities for "AArch64" architecture. It currently
doesn't break any test case, but we should fix it anyway to avoid future
surprises which would be quite hard to debug.
Signed-off-by: Jiri Denemark
---
tests/testutilsqemu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 8438613f28..52ea6bf655 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -575,6 +575,8 @@ qemuTestSetHostCPU(virCapsPtr caps,
setenv("VIR_TEST_MOCK_FAKE_HOST_CPU", cpu->model, 1);
else
unsetenv("VIR_TEST_MOCK_FAKE_HOST_CPU");
+} else {
+unsetenv("VIR_TEST_MOCK_FAKE_HOST_CPU");
}
caps->host.cpu = cpu;
}
--
2.18.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 03:48:13PM +0200, Igor Mammedov wrote:
> -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
> so that total number of logical CPUs [sockets * cores * threads]
> would be equal to [maxcpus], however historically we didn't have
> such check in QEMU and it is possible to start VM with an invalid
> topology.
> Deprecate invalid options combination so we can make sure that
> the topology VM started with is always correct in the future.
> Users with an invalid sockets/cores/threads/maxcpus values should
> fix their CLI to make sure that
>[sockets * cores * threads] == [maxcpus]
>
> Signed-off-by: Igor Mammedov
> ---
> v4:
> - missed dot comment, fix it with s/./,/ (Andrew Jones )
> v3:
> - more spelling fixes (Andrew Jones )
> - place deprecation check after (sockets * cores * threads > max_cpus) check
> (Eduardo Habkost )
> v2:
> - spelling&&co fixes (Andrew Jones )
> ---
> qemu-deprecated.texi | 11 +++
> vl.c | 7 +++
> 2 files changed, 18 insertions(+)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 87212b6..094b4ca 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -159,6 +159,17 @@ The 'file' driver for drives is no longer appropriate
> for character or host
> devices and will only accept regular files (S_IFREG). The correct driver
> for these file types is 'host_cdrom' or 'host_device' as appropriate.
>
> +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs, but historically it was possible to start QEMU with
> +an incorrect topology where
> + sockets * cores * threads >= X && X < maxcpus
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies will be removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> + sockets * cores * threads == maxcpus
> +
> @section QEMU Machine Protocol (QMP) commands
>
> @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> diff --git a/vl.c b/vl.c
> index 5ba06ad..7fd700e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts)
> exit(1);
> }
>
> +if (sockets * cores * threads != max_cpus) {
> +warn_report("Invalid CPU topology deprecated: "
> +"sockets (%u) * cores (%u) * threads (%u) "
> +"!= maxcpus (%u)",
> +sockets, cores, threads, max_cpus);
> +}
> +
> smp_cpus = cpus;
> smp_cores = cores;
> smp_threads = threads;
> --
> 2.7.4
>
>
Reviewed-by: Andrew Jones
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 04:10:53PM +0200, Erik Skultety wrote:
> On Tue, Aug 28, 2018 at 03:52:56PM +0200, Ján Tomko wrote:
> > On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
> > > On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
> > > > On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
> > > > > On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> > > > > > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > > > > > > > So how about storing 2 sets of expected data for this test case.
> > > > > > > >
> > > > > >
> > > > > > Two is not enough. My clang 5.0.1 produces a test that displays the
> > > > > > monkeys correctly, but does not count their width properly:
> > > > >
> > > > > Is this a different bug though ? The issue with iswprint() is varying
> > > > > according to glibc version, not compiler version.
> > > > >
> > > >
> > > > The broken setup is:
> > > > sys-libs/glibc-2.25-r9
> > > > sys-devel/clang-5.0.1
> > > >
> > > > It works on:
> > > > sys-libs/glibc-2.26-r7
> > > > with either of:
> > > > sys-devel/clang-5.0.1
> > > > sys-devel/clang-6.0.1
> > > >
> > > > So yes, it is a glibc bug.
> > > > Depending on the version, either just wcwidth returns incorrect values
> > > > for the monkeys (my case) or iswprint considers them non-printable.
> > >
> > > It sounds like in your case we're genuinely broken in the virsh
> > > code, not merely tests broken.
> > >
> > > I wonder if we need extra logic in the virsh code to handle escaping
> > > for the cases where wcwidth is returning wrong data, so we still get
> > > column layout correct ?
> > >
> >
> > I don't think so.
> >
> > 1) wouldn't that involve reimplementing the wcwidth function?
> > 2) users crazy enough to use new unicode characters are welcome to
> > upgrade to new glibc, or suffer through misaligned virsh tables.
>
> I second that, I don't think that in this case it's any beneficial trying to
> fix glibc problems just to offer users a consistent experience, we're not
> gnulib, besides, we're talking about table alignment which has been broken
> for ages and we've already made a significant progress here. Additionally, as
> Jano has pointed out, if someone feels adventurous, that's fine, but it
> may come with certain limitations.
Ok, so we just need get the test to correctly skip on platforms where
we know it won't get right answers
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 03:52:56PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
> > On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
> > > On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> > > > > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > > > > > > So how about storing 2 sets of expected data for this test case.
> > > > > > >
> > > > >
> > > > > Two is not enough. My clang 5.0.1 produces a test that displays the
> > > > > monkeys correctly, but does not count their width properly:
> > > >
> > > > Is this a different bug though ? The issue with iswprint() is varying
> > > > according to glibc version, not compiler version.
> > > >
> > >
> > > The broken setup is:
> > > sys-libs/glibc-2.25-r9
> > > sys-devel/clang-5.0.1
> > >
> > > It works on:
> > > sys-libs/glibc-2.26-r7
> > > with either of:
> > > sys-devel/clang-5.0.1
> > > sys-devel/clang-6.0.1
> > >
> > > So yes, it is a glibc bug.
> > > Depending on the version, either just wcwidth returns incorrect values
> > > for the monkeys (my case) or iswprint considers them non-printable.
> >
> > It sounds like in your case we're genuinely broken in the virsh
> > code, not merely tests broken.
> >
> > I wonder if we need extra logic in the virsh code to handle escaping
> > for the cases where wcwidth is returning wrong data, so we still get
> > column layout correct ?
> >
>
> I don't think so.
>
> 1) wouldn't that involve reimplementing the wcwidth function?
> 2) users crazy enough to use new unicode characters are welcome to
> upgrade to new glibc, or suffer through misaligned virsh tables.
I second that, I don't think that in this case it's any beneficial trying to
fix glibc problems just to offer users a consistent experience, we're not
gnulib, besides, we're talking about table alignment which has been broken
for ages and we've already made a significant progress here. Additionally, as
Jano has pointed out, if someone feels adventurous, that's fine, but it
may come with certain limitations.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/28/2018 03:30 PM, John Ferlan wrote:
>
>
> On 08/20/2018 08:09 AM, Michal Privoznik wrote:
>> In two places the passed pool object argument is marked as
>> ATTRIBUTE_UNUSED even though it's used right away.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>> src/storage/storage_backend_rbd.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>
> Reviewed-by: John Ferlan
>
> Although freeze hasn't been announced, an rc1 tag exists - so I'll state
> that although the first 2 patches could be designated safe for freeze,
> the rest of the series needs to wait until after 4.7.0.
Well, the whole patch set can be viewed as a bug fix and as such is
exempt from freeze ;-)
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:46:08PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > > > So how about storing 2 sets of expected data for this test case.
> > > >
> >
> > Two is not enough. My clang 5.0.1 produces a test that displays the
> > monkeys correctly, but does not count their width properly:
>
> Is this a different bug though ? The issue with iswprint() is varying
> according to glibc version, not compiler version.
>
The broken setup is:
sys-libs/glibc-2.25-r9
sys-devel/clang-5.0.1
It works on:
sys-libs/glibc-2.26-r7
with either of:
sys-devel/clang-5.0.1
sys-devel/clang-6.0.1
So yes, it is a glibc bug.
Depending on the version, either just wcwidth returns incorrect values
for the monkeys (my case) or iswprint considers them non-printable.
It sounds like in your case we're genuinely broken in the virsh
code, not merely tests broken.
I wonder if we need extra logic in the virsh code to handle escaping
for the cases where wcwidth is returning wrong data, so we still get
column layout correct ?
I don't think so.
1) wouldn't that involve reimplementing the wcwidth function?
2) users crazy enough to use new unicode characters are welcome to
upgrade to new glibc, or suffer through misaligned virsh tables.
Jano
> So I wonder if the clang problem you mention is something that can be
> fixed in some way ?
>
Nope, red herring. My maint point was that there are more than 2
possible results.
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 :|
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
so that total number of logical CPUs [sockets * cores * threads]
would be equal to [maxcpus], however historically we didn't have
such check in QEMU and it is possible to start VM with an invalid
topology.
Deprecate invalid options combination so we can make sure that
the topology VM started with is always correct in the future.
Users with an invalid sockets/cores/threads/maxcpus values should
fix their CLI to make sure that
[sockets * cores * threads] == [maxcpus]
Signed-off-by: Igor Mammedov
---
v4:
- missed dot comment, fix it with s/./,/ (Andrew Jones )
v3:
- more spelling fixes (Andrew Jones )
- place deprecation check after (sockets * cores * threads > max_cpus) check
(Eduardo Habkost )
v2:
- spelling&&co fixes (Andrew Jones )
---
qemu-deprecated.texi | 11 +++
vl.c | 7 +++
2 files changed, 18 insertions(+)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 87212b6..094b4ca 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -159,6 +159,17 @@ The 'file' driver for drives is no longer appropriate for
character or host
devices and will only accept regular files (S_IFREG). The correct driver
for these file types is 'host_cdrom' or 'host_device' as appropriate.
+@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
+
+CPU topology properties should describe whole machine topology including
+possible CPUs, but historically it was possible to start QEMU with
+an incorrect topology where
+ sockets * cores * threads >= X && X < maxcpus
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies will be removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+ sockets * cores * threads == maxcpus
+
@section QEMU Machine Protocol (QMP) commands
@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/vl.c b/vl.c
index 5ba06ad..7fd700e 100644
--- a/vl.c
+++ b/vl.c
@@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts)
exit(1);
}
+if (sockets * cores * threads != max_cpus) {
+warn_report("Invalid CPU topology deprecated: "
+"sockets (%u) * cores (%u) * threads (%u) "
+"!= maxcpus (%u)",
+sockets, cores, threads, max_cpus);
+}
+
smp_cpus = cpus;
smp_cores = cores;
smp_threads = threads;
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 03:41:26PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
> > On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> > > On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > > > > So how about storing 2 sets of expected data for this test case.
> > > > >
> > >
> > > Two is not enough. My clang 5.0.1 produces a test that displays the
> > > monkeys correctly, but does not count their width properly:
> >
> > Is this a different bug though ? The issue with iswprint() is varying
> > according to glibc version, not compiler version.
> >
>
> The broken setup is:
> sys-libs/glibc-2.25-r9
> sys-devel/clang-5.0.1
>
> It works on:
> sys-libs/glibc-2.26-r7
> with either of:
> sys-devel/clang-5.0.1
> sys-devel/clang-6.0.1
>
> So yes, it is a glibc bug.
> Depending on the version, either just wcwidth returns incorrect values
> for the monkeys (my case) or iswprint considers them non-printable.
It sounds like in your case we're genuinely broken in the virsh
code, not merely tests broken.
I wonder if we need extra logic in the virsh code to handle escaping
for the cases where wcwidth is returning wrong data, so we still get
column layout correct ?
> > So I wonder if the clang problem you mention is something that can be
> > fixed in some way ?
> >
>
> Nope, red herring. My maint point was that there are more than 2
> possible results.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:24:19PM +0100, Daniel P. Berrangé wrote:
On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > So how about storing 2 sets of expected data for this test case.
> >
Two is not enough. My clang 5.0.1 produces a test that displays the
monkeys correctly, but does not count their width properly:
Is this a different bug though ? The issue with iswprint() is varying
according to glibc version, not compiler version.
The broken setup is:
sys-libs/glibc-2.25-r9
sys-devel/clang-5.0.1
It works on:
sys-libs/glibc-2.26-r7
with either of:
sys-devel/clang-5.0.1
sys-devel/clang-6.0.1
So yes, it is a glibc bug.
Depending on the version, either just wcwidth returns incorrect values
for the monkeys (my case) or iswprint considers them non-printable.
So I wonder if the clang problem you mention is something that can be
fixed in some way ?
Nope, red herring. My maint point was that there are more than 2
possible results.
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 03:26:07PM +0200, Igor Mammedov wrote:
> -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
> so that total number of logical CPUs [sockets * cores * threads]
> would be equal to [maxcpus], however historically we didn't have
> such check in QEMU and it is possible to start VM with an invalid
> topology.
> Deprecate invalid options combination so we can make sure that
> the topology VM started with is always correct in the future.
> Users with an invalid sockets/cores/threads/maxcpus values should
> fix their CLI to make sure that
>[sockets * cores * threads] == [maxcpus]
>
> Signed-off-by: Igor Mammedov
> ---
> v3:
> - more spelling fixes (Andrew Jones )
> - place deprecation check after (sockets * cores * threads > max_cpus) check
> (Eduardo Habkost )
> v2:
> - spelling&&co fixes (Andrew Jones )
> ---
> qemu-deprecated.texi | 11 +++
> vl.c | 7 +++
> 2 files changed, 18 insertions(+)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 87212b6..dc9d9ee 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -159,6 +159,17 @@ The 'file' driver for drives is no longer appropriate
> for character or host
> devices and will only accept regular files (S_IFREG). The correct driver
> for these file types is 'host_cdrom' or 'host_device' as appropriate.
>
> +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs. but historically it was possible to start QEMU with
Change this period (.) to a comma (,) please :-)
> +an incorrect topology where
> + sockets * cores * threads >= X && X < maxcpus
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies will be removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> + sockets * cores * threads == maxcpus
> +
> @section QEMU Machine Protocol (QMP) commands
>
> @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> diff --git a/vl.c b/vl.c
> index 5ba06ad..7fd700e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts)
> exit(1);
> }
>
> +if (sockets * cores * threads != max_cpus) {
> +warn_report("Invalid CPU topology deprecated: "
> +"sockets (%u) * cores (%u) * threads (%u) "
> +"!= maxcpus (%u)",
> +sockets, cores, threads, max_cpus);
> +}
> +
> smp_cpus = cpus;
> smp_cores = cores;
> smp_threads = threads;
> --
> 2.7.4
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> This function is going to be made static in used in
> virStoragePoolObjAssignDef(). Therefore move it a few lines up.
>
> Signed-off-by: Michal Privoznik
> ---
> src/conf/virstorageobj.c | 124
> +++
> 1 file changed, 62 insertions(+), 62 deletions(-)
>
Reviewed-by: John Ferlan
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/20/2018 08:09 AM, Michal Privoznik wrote:
> In two places the passed pool object argument is marked as
> ATTRIBUTE_UNUSED even though it's used right away.
>
> Signed-off-by: Michal Privoznik
> ---
> src/storage/storage_backend_rbd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: John Ferlan
Although freeze hasn't been announced, an rc1 tag exists - so I'll state
that although the first 2 patches could be designated safe for freeze,
the rest of the series needs to wait until after 4.7.0.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
so that total number of logical CPUs [sockets * cores * threads]
would be equal to [maxcpus], however historically we didn't have
such check in QEMU and it is possible to start VM with an invalid
topology.
Deprecate invalid options combination so we can make sure that
the topology VM started with is always correct in the future.
Users with an invalid sockets/cores/threads/maxcpus values should
fix their CLI to make sure that
[sockets * cores * threads] == [maxcpus]
Signed-off-by: Igor Mammedov
---
v3:
- more spelling fixes (Andrew Jones )
- place deprecation check after (sockets * cores * threads > max_cpus) check
(Eduardo Habkost )
v2:
- spelling&&co fixes (Andrew Jones )
---
qemu-deprecated.texi | 11 +++
vl.c | 7 +++
2 files changed, 18 insertions(+)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 87212b6..dc9d9ee 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -159,6 +159,17 @@ The 'file' driver for drives is no longer appropriate for
character or host
devices and will only accept regular files (S_IFREG). The correct driver
for these file types is 'host_cdrom' or 'host_device' as appropriate.
+@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
+
+CPU topology properties should describe whole machine topology including
+possible CPUs. but historically it was possible to start QEMU with
+an incorrect topology where
+ sockets * cores * threads >= X && X < maxcpus
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies will be removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+ sockets * cores * threads == maxcpus
+
@section QEMU Machine Protocol (QMP) commands
@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/vl.c b/vl.c
index 5ba06ad..7fd700e 100644
--- a/vl.c
+++ b/vl.c
@@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts)
exit(1);
}
+if (sockets * cores * threads != max_cpus) {
+warn_report("Invalid CPU topology deprecated: "
+"sockets (%u) * cores (%u) * threads (%u) "
+"!= maxcpus (%u)",
+sockets, cores, threads, max_cpus);
+}
+
smp_cpus = cpus;
smp_cores = cores;
smp_threads = threads;
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > > > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > > > >
> > > > > > But first fix the build failures :-)
> > > > > >
> > > > > > On CentOS / RHEL:
> > > > > >
> > > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > > > >
> > > > > >
> > > > > > 4)
> > > > > > testUnicode .
> > > > > > ..
> > > > > > Offset 30
> > > > > > Expect [государство
> > > > > > -
> > > > > > 1fedora28 running
> > > > > > 2🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > > > Actual
> > > > > > [
> > > > > > государство
> > > > > > -
> > > > > >
> > > > > > 1fedora28
> > > > > > running
> > > > > > 2\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > > > >
> > > > >
> > > > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > > > line.
> > > > > Haven't tested it though.
> > > >
> > > > I tried but it didn't help. From what I understood, CentOS has problems
> > > > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > > > any of those characters to wchar_t successfully and properly, but when
> > > > we pass that character to iswprint, it returns 0 (considers those wide
> > > > characters nonprintable).
> > >
> > > On the plus side, it appears that when this problem hits, the code is
> > > still correctly doing the column alignment taking account of these
> > > unexpected escape sequences.
> > >
> > > So how about storing 2 sets of expected data for this test case.
> > >
>
> Two is not enough. My clang 5.0.1 produces a test that displays the
> monkeys correctly, but does not count their width properly:
Is this a different bug though ? The issue with iswprint() is varying
according to glibc version, not compiler version.
So I wonder if the clang problem you mention is something that can be
fixed in some way ?
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> > On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > > >
> > > > > But first fix the build failures :-)
> > > > >
> > > > > On CentOS / RHEL:
> > > > >
> > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > > >
> > > > >
> > > > > 4)
> > > > > testUnicode .
> > > > > ..
> > > > > Offset 30
> > > > > Expect [государство
> > > > > -
> > > > > 1fedora28 running
> > > > > 2🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > > Actual
> > > > > [
> > > > > государство
> > > > > -
> > > > >
> > > > > 1fedora28
> > > > > running
> > > > > 2\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > > >
> > > >
> > > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > > line.
> > > > Haven't tested it though.
> > >
> > > I tried but it didn't help. From what I understood, CentOS has problems
> > > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > > any of those characters to wchar_t successfully and properly, but when
> > > we pass that character to iswprint, it returns 0 (considers those wide
> > > characters nonprintable).
> >
> > On the plus side, it appears that when this problem hits, the code is
> > still correctly doing the column alignment taking account of these
> > unexpected escape sequences.
> >
> > So how about storing 2 sets of expected data for this test case.
> >
> > In the unit test then call iswprint() to figure out which of the
> > two expected data sets to compare against.
>
> How does it help us during runtime when someone uses such characters in a
> domain's name? It would still return a row consisting of escape sequences. So
> what's the point of providing 2 sets of expected data for a test just so it
> can
> pass, rather than use unicode characters we know would pass and everything
> else
> is a platform limitation which is out of our hands.
Well the idea is to prove that we get the column width calculation
correct, even when the the glibc we have doesn't support these as
printable characters
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Peter Krempa
---
v2:
- switched from '$SECTION' to '$MODULE' as a hint for the split files
- changed 'domain.rng' to 'domaincommon.rng' which contains the defs
- added the '$MODULE' to virsh path
Note some still may be wrong as I did not chec all paths rigorously.
docs/api_extension.html.in | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/docs/api_extension.html.in b/docs/api_extension.html.in
index 33d82e6ac6..d7fbbd6e90 100644
--- a/docs/api_extension.html.in
+++ b/docs/api_extension.html.in
@@ -96,7 +96,7 @@
schema and document the new elements or attributes:
-docs/schemas/domain.rng
+docs/schemas/domaincommon.rng
docs/formatdomain.html.in
@@ -106,7 +106,7 @@
libvirt library and call the new function:
-include/libvirt/libvirt.h.in
+include/libvirt/libvirt-$MODULE.h.in
src/libvirt_public.syms
@@ -137,7 +137,7 @@
The driver structs are defined in:
-src/driver.h
+src/driver-$MODULE.h
To define the internal API, first typedef the driver function
@@ -177,7 +177,7 @@
The public API calls are implemented in:
-src/libvirt.c
+src/libvirt-$MODULE.c
Implementing the remote protocol
@@ -219,9 +219,9 @@
-daemon/remote_dispatch_args.h
-daemon/remote_dispatch_prototypes.h
-daemon/remote_dispatch_table.h
+src/remote/remote_daemon_dispatch_stubs.h
+src/remote/remote_daemon_dispatch.h
+src/remote/remote_daemon_dispatch.c
src/remote/remote_protocol.c
src/remote/remote_protocol.h
@@ -233,7 +233,7 @@
method calls go in:
-src/remote/remote_internal.c
+src/remote/remote_driver.c
Each remote method invocation does the following:
@@ -256,7 +256,7 @@
The server side dispatchers are implemented in:
-daemon/remote.c
+src/remote/daemon_dispatch.c
Again, this step uses the .h files generated by make rpcgen.
@@ -309,7 +309,7 @@
-tools/virsh.c
+tools/virsh-$MODULE.c
tools/virsh.pod
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2018-08-28 at 14:35 +0200, Simon Kobyda wrote:
> CentOS 7 uses
> glibc 2.17, which was released in December 2017
Sorry made a typo there, glibc 2.17 was released in 2012
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:30:23PM +0200, Erik Skultety wrote:
On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
I still see a benefit in having testUnicodeBasic that passes everywhere
(does it?), and conditionally running the monkey test on platforms where
iswprint returns the proper results.
Why? To see that glibc is new enough to support it? One would assume
Then one might be surprised one day.
Jano
that if it
works for n (given your example I'm so sure it actually does...), it would work
for n+1 too, so I still don't see the point in this specific test case.
Thanks for the example though.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:24:42PM +0200, Ján Tomko wrote:
> On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
> > On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > > > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > > > >
> > > > > > But first fix the build failures :-)
> > > > > >
> > > > > > On CentOS / RHEL:
> > > > > >
> > > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > > > >
> > > > > >
> > > > > > 4)
> > > > > > testUnicode .
> > > > > > ..
> > > > > > Offset 30
> > > > > > Expect [государство
> > > > > > -
> > > > > > 1fedora28 running
> > > > > > 2🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > > > Actual
> > > > > > [
> > > > > > государство
> > > > > > -
> > > > > >
> > > > > > 1fedora28
> > > > > > running
> > > > > > 2\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > > > >
> > > > >
> > > > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > > > line.
> > > > > Haven't tested it though.
> > > >
> > > > I tried but it didn't help. From what I understood, CentOS has problems
> > > > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > > > any of those characters to wchar_t successfully and properly, but when
> > > > we pass that character to iswprint, it returns 0 (considers those wide
> > > > characters nonprintable).
> > >
> > > On the plus side, it appears that when this problem hits, the code is
> > > still correctly doing the column alignment taking account of these
> > > unexpected escape sequences.
> > >
> > > So how about storing 2 sets of expected data for this test case.
> > >
>
> Two is not enough. My clang 5.0.1 produces a test that displays the
> monkeys correctly, but does not count their width properly:
>
> $ VIR_TEST_RANGE=4 VIR_TEST_DEBUG=1 ./run tests/vshtabletest
> TEST: vshtabletest
> 4) testUnicode ...
> Offset 24
> Expect [ государство
> -
> 1fedora28 ]
> Actual [государство
> ---
> 1fedora28]
> ...
> FAILED
> > > In the unit test then call iswprint() to figure out which of the
> > > two expected data sets to compare against.
> >
> > How does it help us during runtime when someone uses such characters in a
> > domain's name? It would still return a row consisting of escape sequences.
>
> Not necessarily, see above.
>
> > So
> > what's the point of providing 2 sets of expected data for a test just so it
> > can
> > pass, rather than use unicode characters we know would pass and everything
> > else
> > is a platform limitation which is out of our hands.
>
> I still see a benefit in having testUnicodeBasic that passes everywhere
> (does it?), and conditionally running the monkey test on platforms where
> iswprint returns the proper results.
Why? To see that glibc is new enough to support it? One would assume that if it
works for n (given your example I'm so sure it actually does...), it would work
for n+1 too, so I still don't see the point in this specific test case.
Thanks for the example though.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 02:10:55PM +0200, Erik Skultety wrote:
On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > >
> > > But first fix the build failures :-)
> > >
> > > On CentOS / RHEL:
> > >
> > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > >
> > >
> > > 4)
> > > testUnicode .
> > > ..
> > > Offset 30
> > > Expect [государство
> > > -
> > > 1fedora28 running
> > > 2🙊🙉🙈rhel7.5🙆🙆🙅]
> > > Actual
> > > [
> > > государство
> > > -
> > >
> > > 1fedora28
> > > running
> > > 2\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > >
> >
> > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > supposed to be fixed by adding -finput-charset= onto gcc command
> > line.
> > Haven't tested it though.
>
> I tried but it didn't help. From what I understood, CentOS has problems
> with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> any of those characters to wchar_t successfully and properly, but when
> we pass that character to iswprint, it returns 0 (considers those wide
> characters nonprintable).
On the plus side, it appears that when this problem hits, the code is
still correctly doing the column alignment taking account of these
unexpected escape sequences.
So how about storing 2 sets of expected data for this test case.
Two is not enough. My clang 5.0.1 produces a test that displays the
monkeys correctly, but does not count their width properly:
$ VIR_TEST_RANGE=4 VIR_TEST_DEBUG=1 ./run tests/vshtabletest
TEST: vshtabletest
4) testUnicode ...
Offset 24
Expect [ государство
-
1fedora28 ]
Actual [государство
---
1fedora28]
... FAILED
In the unit test then call iswprint() to figure out which of the
two expected data sets to compare against.
How does it help us during runtime when someone uses such characters in a
domain's name? It would still return a row consisting of escape sequences.
Not necessarily, see above.
So
what's the point of providing 2 sets of expected data for a test just so it can
pass, rather than use unicode characters we know would pass and everything else
is a platform limitation which is out of our hands.
I still see a benefit in having testUnicodeBasic that passes everywhere
(does it?), and conditionally running the monkey test on platforms where
iswprint returns the proper results.
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 11:35:02AM +0100, Daniel P. Berrangé wrote:
> On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> > On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > > >
> > > > But first fix the build failures :-)
> > > >
> > > > On CentOS / RHEL:
> > > >
> > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > > >
> > > >
> > > > 4)
> > > > testUnicode .
> > > > ..
> > > > Offset 30
> > > > Expect [государство
> > > > -
> > > > 1fedora28 running
> > > > 2🙊🙉🙈rhel7.5🙆🙆🙅]
> > > > Actual
> > > > [
> > > > государство
> > > > -
> > > >
> > > > 1fedora28
> > > > running
> > > > 2\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > > >
> > >
> > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > > supposed to be fixed by adding -finput-charset= onto gcc command
> > > line.
> > > Haven't tested it though.
> >
> > I tried but it didn't help. From what I understood, CentOS has problems
> > with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> > any of those characters to wchar_t successfully and properly, but when
> > we pass that character to iswprint, it returns 0 (considers those wide
> > characters nonprintable).
>
> On the plus side, it appears that when this problem hits, the code is
> still correctly doing the column alignment taking account of these
> unexpected escape sequences.
>
> So how about storing 2 sets of expected data for this test case.
>
> In the unit test then call iswprint() to figure out which of the
> two expected data sets to compare against.
How does it help us during runtime when someone uses such characters in a
domain's name? It would still return a row consisting of escape sequences. So
what's the point of providing 2 sets of expected data for a test just so it can
pass, rather than use unicode characters we know would pass and everything else
is a platform limitation which is out of our hands.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Aug 24, 2018 at 14:40:08 -0600, Jim Fehlig wrote:
> While investigating a bug [1] found by Xen's osstest I realized I don't quite
> understand how to handle modify jobs (e.g. BeginJob/EndJob) on virDomainObj
> across the various phases of V3 migration protocol. E.g. on the src host the
> Begin, Perform, and Confirm phases are performed. Should a modify job start
> (BeginJob) in the Begin phase and stop (EndJob) in the Confirm phase? Or
> should
> each phase, if necessary, do BeginJob/EndJob? Same question for dst host. IMO
> the job should be held across the phases on each host, preventing any
> modifications during the overall migration process.
Right, the first phase (Begin on the source and Prepare on the
destination) should acquire the job and it should be held until the end
of the migration (Confirm/Finish) to make sure nothing changes during
the migration. In QEMU driver, we have several helpers around the
generic job APIs:
qemuMigrationJobStart
- used at the beginning of migration
qemuMigrationJobSetPhase
- called at the beginning of each migration phase except the
first one (the first one calls qemuMigrationJobStart)
qemuMigrationJobContinue
- called at the end of each phase except for the last one (which
calls qemuMigrationJobFinish)
qemuMigrationJobFinish
- called at the end of migration
> Although I do worry about orphaned jobs, e.g. a missed EndJob caused
> by some obscure error in the migration machinery.
Well, this could happen even if the job was acquired by each step
separately. I don't think that spanning the job over several APIs makes
the situation significantly worse.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Aug 28, 2018 at 12:45:27 +0200, Erik Skultety wrote:
> On Mon, Aug 27, 2018 at 06:17:05PM +0200, Peter Krempa wrote:
> > Add a new modifier letter for virJSONValueObjectAddVArgs which will add
> > a boolean value with our tristate semantics. The value is omitted when
> > the _ABSENT value is used.
> >
> > Signed-off-by: Peter Krempa
> > ---
> > src/util/virjson.c | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/src/util/virjson.c b/src/util/virjson.c
> > index 29530dcb15..e45f1fab06 100644
> > --- a/src/util/virjson.c
> > +++ b/src/util/virjson.c
> > @@ -142,6 +142,8 @@ virJSONValueGetType(const virJSONValue *value)
> > *
> > * b: boolean value
> > * B: boolean value, omitted if false
> > + * T: boolean value specified by a virTristate(Bool|Switch) value, omitted
> > on
> > + * the _ABSENT value
> > *
> > * d: double precision floating point number
> > * n: json null value
> > @@ -266,12 +268,23 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
> > } break;
> >
> > case 'B':
> > +case 'T':
> > case 'b': {
> > int val = va_arg(args, int);
> >
> > if (!val && type == 'B')
> > continue;
> >
> > +if (type == 'T') {
> > +if (val == VIR_TRISTATE_SWITCH_ABSENT)
>
> Just curious, was ^this on purpose to emphasize the fact that it would really
> work with TristateSwitch since both _ABSENTs == 0 or just a coincidence? I'd
> probably prefer _SWITCH_ABSENT slightly more to be consistent with the enum
> being tested below.
No, that was a mistake. I'll fix it before pushing. I'll also order the
'B' and 'b' cases together.
>
> Reviewed-by: Erik Skultety
>
> > +continue;
> > +
> > +if (val == VIR_TRISTATE_BOOL_NO)
> > +val = 0;
> > +else
> > +val = 1;
> > +}
> > +
> > rc = virJSONValueObjectAppendBoolean(obj, key, val);
> > } break;
> >
> > --
> > 2.16.2
> >
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Aug 27, 2018 at 06:17:05PM +0200, Peter Krempa wrote:
> Add a new modifier letter for virJSONValueObjectAddVArgs which will add
> a boolean value with our tristate semantics. The value is omitted when
> the _ABSENT value is used.
>
> Signed-off-by: Peter Krempa
> ---
> src/util/virjson.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 29530dcb15..e45f1fab06 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -142,6 +142,8 @@ virJSONValueGetType(const virJSONValue *value)
> *
> * b: boolean value
> * B: boolean value, omitted if false
> + * T: boolean value specified by a virTristate(Bool|Switch) value, omitted on
> + * the _ABSENT value
> *
> * d: double precision floating point number
> * n: json null value
> @@ -266,12 +268,23 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
> } break;
>
> case 'B':
> +case 'T':
> case 'b': {
> int val = va_arg(args, int);
>
> if (!val && type == 'B')
> continue;
>
> +if (type == 'T') {
> +if (val == VIR_TRISTATE_SWITCH_ABSENT)
Just curious, was ^this on purpose to emphasize the fact that it would really
work with TristateSwitch since both _ABSENTs == 0 or just a coincidence? I'd
probably prefer _SWITCH_ABSENT slightly more to be consistent with the enum
being tested below.
Reviewed-by: Erik Skultety
> +continue;
> +
> +if (val == VIR_TRISTATE_BOOL_NO)
> +val = 0;
> +else
> +val = 1;
> +}
> +
> rc = virJSONValueObjectAppendBoolean(obj, key, val);
> } break;
>
> --
> 2.16.2
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2018-08-28 at 12:24 +0200, Ján Tomko wrote:
> Commit deb057f added a switch without a default case.
> Add it and call virReportEnumRangeError for _LAST too.
>
> Signed-off-by: Ján Tomko
> ---
> src/conf/domain_conf.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: Andrea Bolognani
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Aug 27, 2018 at 05:50:22PM +0200, Simon Kobyda wrote:
> On Fri, 2018-08-24 at 12:10 +0200, Michal Privoznik wrote:
> > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote:
> > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote:
> > >
> > > But first fix the build failures :-)
> > >
> > > On CentOS / RHEL:
> > >
> > > https://travis-ci.org/libvirt/libvirt/jobs/420024141
> > >
> > >
> > > 4)
> > > testUnicode .
> > > ..
> > > Offset 30
> > > Expect [государство
> > > -
> > > 1fedora28 running
> > > 2🙊🙉🙈rhel7.5🙆🙆🙅]
> > > Actual
> > > [
> > > государство
> > > -
> > >
> > > 1fedora28
> > > running
> > > 2\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\x
> > > ff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff]
> > >
> >
> > Okay, this is probably due to ancient gcc that's there (4.8.0) and is
> > supposed to be fixed by adding -finput-charset= onto gcc command
> > line.
> > Haven't tested it though.
>
> I tried but it didn't help. From what I understood, CentOS has problems
> with unicodes such as 🙊🙉🙈🙆🙆🙅. On that system, it can convert
> any of those characters to wchar_t successfully and properly, but when
> we pass that character to iswprint, it returns 0 (considers those wide
> characters nonprintable).
On the plus side, it appears that when this problem hits, the code is
still correctly doing the column alignment taking account of these
unexpected escape sequences.
So how about storing 2 sets of expected data for this test case.
In the unit test then call iswprint() to figure out which of the
two expected data sets to compare against.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
{ "BLURB": "HERE" }
Ján Tomko (5):
virjsontest: store name in testInfo
virjsontest: use name instead of doc for deflatten test
virjsontest: use the test name in AddRemove test
Test parsing of large numbers in JSON
tests: pass ULLONG_MAX to qemuMonitorJSONGetBalloonInfo
tests/qemumonitorjsontest.c | 4 ++--
tests/virjsontest.c | 22 +-
2 files changed, 15 insertions(+), 11 deletions(-)
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
We expect to get numbers as big as ULLONG_MAX from QEMU,
add a test for them.
Signed-off-by: Ján Tomko
---
tests/virjsontest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/virjsontest.c b/tests/virjsontest.c
index 195d8279f0..baf1070c5a 100644
--- a/tests/virjsontest.c
+++ b/tests/virjsontest.c
@@ -589,6 +589,9 @@ mymain(void)
DO_TEST_PARSE("float without garbage", "[ 1.024e19 ]", "[1.024e19]");
DO_TEST_PARSE_FAIL("float with garbage", "[ 0.0314159ee+100 ]");
+DO_TEST_PARSE("unsigned minus one", "[ 18446744073709551615 ]",
"[18446744073709551615]");
+DO_TEST_PARSE("another big number", "[ 9223372036854775808 ]",
"[9223372036854775808]");
+
DO_TEST_PARSE("string", "[ \"The meaning of life\" ]",
"[\"The meaning of life\"]");
DO_TEST_PARSE_FAIL("unterminated string", "[ \"The meaning of lif ]");
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Aug 27, 2018 at 06:09:08PM +0200, Peter Krempa wrote:
> Now we assume the flag always so there's no use for this test. Probably
> a leftover from the cleanup of the capability.
>
> Signed-off-by: Peter Krempa
> ---
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/27/2018 04:29 PM, John Ferlan wrote:
>
>
> On 08/27/2018 09:29 AM, Michal Prívozník wrote:
>> On 08/27/2018 12:57 PM, Yi Wang wrote:
>>> When doing some job holding state lock for a long time,
>>> we may come across error:
>>> "Timed out during operation: cannot acquire state change lock"
>>> Well, sometimes it's not a problem and users wanner continue
>
> s/wanner/want to/
>
> or "prefer to"... Perhaps users is too vague - this essentially sets
> the "default timeout policy" for everyone. Setting it too short could be
> dangerous considering it's impact.
>
> If "a user" wanted or preferred to wait longer than the default job
> timeout for a specific command, then we'd need to add some option for
> various commands/API's that allowed setting a unique timeout value for a
> specific job.
>
> Another thought would be to have an API that allowed changing the
> timeout programmatically. Of course there's the admin interface that is
> another area to consider altering since you're adjusting a configuration
> value like this.
If anything, this is a job for virt-admin. I don't think we need an API
that would live next to virDomainDeviceUpdate() for instance. We don't
have one for max_jobs, nor for lock_manager, ... In fact, I don't think
we have any API to change anything from qemu.conf.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Aug 27, 2018 at 06:09:07PM +0200, Peter Krempa wrote:
> The test files were unused, but we don't have any other test for this
> feature. Make use of the existing files by removing disks and using
> DO_TEST_CAPS_LATEST to execute them. The legacy output files will be
> dropped.
>
> Signed-off-by: Peter Krempa
> ---
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, 2018-08-27 at 18:09 +0200, Peter Krempa wrote:
> 'metadata' and 'leases' are features internal to libvirt and thus don't
> influence the XML. As they are not tested we don't need the output
> files.
I think you meant s/XML/generated QEMU command line/ here?
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
