Markus Armbruster writes:
> This series extends QMP introspection to cover deprecation.
> Additionally, new option -compat lets you configure what to do when
> deprecated interfaces get used. This is intended for testing users of
> the management interfaces. It is experimental.
>
> -compat deprecated-input= configures what to do when
> deprecated input is received. Available policies:
>
> * accept: Accept deprecated commands and arguments (default)
> * reject: Reject them
> * crash: Crash
>
> -compat deprecated-output= configures what to do when
> deprecated output is sent. Available output policies:
>
> * accept: Emit deprecated command results and events (default)
> * hide: Suppress them
>
> For now, -compat covers only deprecated syntactic aspects of QMP. We
> may want to extend it to cover semantic aspects, CLI, and experimental
> features.
>
> PATCH 01-04: Documentation fixes
> PATCH 05-10: Test improvements
> PATCH 11-24: Add feature flags to remaining user-defined types and to
>struct members
> PATCH 25-26: New special feature 'deprecated', visible in
>introspection
> PATCH 27-34: New -compat to set policy for handling stuff marked with
>feature 'deprecated'
Queued PATCH 01-26. Thanks for the reviews that made this possible!
PATCH 27-34 will have to wait for 5.1.
Ping
Also CC'ing Shiva since he implemented the QEMU side of this feature and
might be interested in it.
Shiva, mind tossing a review in patch 3/5? This is the one that contains
the alignment logic and I'd like to be certain that I didn't mess it up.
Thanks,
DHB
On 3/11/20 6:29 PM, Daniel Henrique Barboza wrote:
changes in v2, all of them affecting just pSeries guests:
- added 'label' requirement
- added code to align down the NVDIMM device
previous version:
https://www.redhat.com/archives/libvir-list/2020-March/msg00165.html
This patch series adds NVDIMM suport for ppc64 guests,
which consists on adding an extra 'uuid' element in
the nvdimm command line and the target label size must
always be provided in the memory definition.
No changes were made in the existing NVDIMM support for
x86 and other archs.
Daniel Henrique Barboza (5):
conf: Introduce optional 'uuid' element for NVDIMM memory
formatdomain.html.in: document the new 'uuid' NVDIMM element
conf, qemu: enable NVDIMM support for ppc64
formatdomain.html.in: document NVDIMM 'label' requirement for pSeries
news.xml: document the new NVDIMM support for Pseries guests
docs/formatdomain.html.in | 24 +++--
docs/news.xml | 11
docs/schemas/domaincommon.rng | 5 ++
src/conf/domain_conf.c| 44 ++--
src/conf/domain_conf.h| 3 ++
src/qemu/qemu_command.c | 7 +++
src/qemu/qemu_domain.c| 47 +++--
.../memory-hotplug-nvdimm-ppc64.args | 32
.../memory-hotplug-nvdimm-ppc64.xml | 50 +++
tests/qemuxml2argvtest.c | 4 ++
.../memory-hotplug-nvdimm-ppc64.xml | 50 +++
tests/qemuxml2xmltest.c | 2 +
12 files changed, 268 insertions(+), 11 deletions(-)
create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.args
create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-ppc64.xml
create mode 100644 tests/qemuxml2xmloutdata/memory-hotplug-nvdimm-ppc64.xml
On 17. 3. 2020 17:39, Julio Faracco wrote:
> I'm good with that.
> I verified other resources that use rtc... they are checking for rtc0.
> Thanks, Michal
>
Cool, I've fixed patches (actually I did slightly change them to make it
easier to add new timer) and pushed.
Reviewed-by: Michal Privoznik
Michal
Introduce new 'multidevs' option for filesystem.
This option prevents misbheaviours on guest if a 9pfs export
contains multiple devices, due to the potential file ID collisions
this otherwise may cause.
Signed-off-by: Christian Schoenebeck
---
docs/formatdomain.html.in | 47 ++-
docs/schemas/domaincommon.rng | 10
src/conf/domain_conf.c| 30 ++
src/conf/domain_conf.h| 13 ++
src/qemu/qemu_command.c | 7 ++
5 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 594146009d..13c506988b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3967,7 +3967,7 @@
-
+
@@ -4084,13 +4084,58 @@
+
Since 5.2.0, the filesystem element
has an optional attribute model with supported values
"virtio-transitional", "virtio-non-transitional", or "virtio".
See Virtio transitional devices
for more details.
+
+
+
+ The filesystem element has an optional attribute multidevs
+ which specifies how to deal with a filesystem export containing more than
+ one device, in order to avoid file ID collisions on guest when using 9pfs
+ (since 6.2.0, requires QEMU 4.2).
+ This attribute is not available for virtiofs. The possible values are:
+
+
+
+default
+
+Use QEMU's default setting (which currently is warn).
+
+remap
+
+This setting allows guest to access multiple devices per export without
+encountering misbehaviours. Inode numbers from host are automatically
+remapped on guest to actively prevent file ID collisions if guest
+accesses one export containing multiple devices.
+
+forbid
+
+Only allow to access one device per export by guest. Attempts to access
+additional devices on the same export will cause the individual
+filesystem access by guest to fail with an error and being logged
(once)
+as error on host side.
+
+warn
+
+This setting resembles the behaviour of 9pfs prior to QEMU 4.2, that is
+no action is performed to prevent any potential file ID collisions if
an
+export contains multiple devices, with the only exception: a warning is
+logged (once) on host side now. This setting may lead to misbehaviours
+on guest side if more than one device is exported per export, due to
the
+potential file ID collisions this may cause on guest side in that case.
+
+
+
+
+ The filesystem element may contain the following
subelements:
+
+
driver
The optional driver element allows specifying further details
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6805420451..9b37740e30 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2676,6 +2676,16 @@
+
+
+
+ default
+ remap
+ forbid
+ warn
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 71535f53f5..b96f87063a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -501,6 +501,14 @@ VIR_ENUM_IMPL(virDomainFSModel,
"virtio-non-transitional",
);
+VIR_ENUM_IMPL(virDomainFSMultidevs,
+ VIR_DOMAIN_FS_MULTIDEVS_LAST,
+ "default",
+ "remap",
+ "forbid",
+ "warn",
+);
+
VIR_ENUM_IMPL(virDomainFSCacheMode,
VIR_DOMAIN_FS_CACHE_MODE_LAST,
"default",
@@ -11376,6 +11384,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *usage = NULL;
g_autofree char *units = NULL;
g_autofree char *model = NULL;
+g_autofree char *multidevs = NULL;
ctxt->node = node;
@@ -11414,6 +11423,17 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
+multidevs = virXMLPropString(node, "multidevs");
+if (multidevs) {
+if ((def->multidevs = virDomainFSMultidevsTypeFromString(multidevs)) <
0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown multidevs '%s'"), multidevs);
+goto error;
+}
+} else {
+def->multidevs = VIR_DOMAIN_FS_MULTIDEVS_DEFAULT;
+}
+
if (virDomainPar
QEMU 4.2 added a new option 'multidevs' for 9pfs. The following patch adds
support for this new option to libvirt.
In short, what is this about: to distinguish files uniquely from each other
in general, numeric file IDs are typically used for comparison, which in
practice is the combination of a file's device ID and the file's inode
number. Unfortunately 9p protocol's QID field used for this purpose,
currently is too small to fit both the device ID and inode number in, which
hence is a problem if one 9pfs export contains multiple devices and may
thus lead to misbheaviours on guest (e.g. with SAMBA file servers) in that
case due to potential file ID collisions.
To mitigate this problem with 9pfs a 'multidevs' option was introduced in
QEMU 4.2 for defining how to deal with this, e.g. multidevs=remap will cause
QEMU's 9pfs implementation to remap all inodes from host side to different
inode numbers on guest side in a way that prevents file ID collisions.
NOTE: In the libvirt docs changes of this libvirt patch I simply assumed
"since 6.2.0". So the final libvirt version number would need to be adjusted
in that text if necessary.
See QEMU discussion with following Message-ID for details:
8a2ffe17fda3a86b9a5a437e1245276881f1e235.1567680121.git.qemu_...@crudebyte.com
Christian Schoenebeck (1):
conf: qemu 9pfs: add 'multidevs' option
docs/formatdomain.html.in | 47 ++-
docs/schemas/domaincommon.rng | 10
src/conf/domain_conf.c| 30 ++
src/conf/domain_conf.h| 13 ++
src/qemu/qemu_command.c | 7 ++
5 files changed, 106 insertions(+), 1 deletion(-)
--
2.20.1
The loop which checks whether the vcpus are in proper configuration for
the requested hot(un)plug skips the first modified vcpu. This means
that 'firstvcpu' which is used to print the error message in case the
configuration is not suitable would never point to the first modified
vcpu.
In cases such as:
8
# virsh setvcpu --config --disable upstream 1
error: invalid argument: vcpu '-1' can't be modified as it is followed by
non-hotpluggable online vcpus
After this fix the proper vcpu is reported in the error message:
# virsh setvcpu --config --disable upstream 1
error: invalid argument: vcpu '1' can't be modified as it is followed by
non-hotpluggable online vcpu
https://bugzilla.redhat.com/show_bug.cgi?id=1611061
Signed-off-by: Peter Krempa
---
src/qemu/qemu_hotplug.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 47069be900..a76df64a7b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -6478,18 +6478,16 @@ qemuDomainVcpuValidateConfig(virDomainDefPtr def,
return -1;
}
+firstvcpu = virBitmapNextSetBit(map, -1);
+
/* non-hotpluggable vcpus need to stay clustered starting from vcpu 0 */
-for (next = virBitmapNextSetBit(map, -1) + 1; next < maxvcpus; next++) {
+for (next = firstvcpu + 1; next < maxvcpus; next++) {
if (!(vcpu = virDomainDefGetVcpu(def, next)))
continue;
/* skip vcpus being modified */
-if (virBitmapIsBitSet(map, next)) {
-if (firstvcpu < 0)
-firstvcpu = next;
-
+if (virBitmapIsBitSet(map, next))
continue;
-}
if (vcpu->online && vcpu->hotpluggable == VIR_TRISTATE_BOOL_NO) {
virReportError(VIR_ERR_INVALID_ARG,
--
2.24.1
On a Tuesday in 2020, Pino Toscano wrote:
It seems like CD-ROMs may have no 'fileName' property specified in case
there is nothing configured as attachment for the drive. Hence, make
sure that virVMXParseDisk() do not consider it mandatory anymore,
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
used also to parse disk and floppies, so make sure that a NULL fileName
is handled in cdrom-related paths.
https://bugzilla.redhat.com/show_bug.cgi?id=1808610
Signed-off-by: Pino Toscano
---
src/vmx/vmx.c | 22 ++
.../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx | 4
.../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 23 +++
tests/vmx2xmltest.c | 1 +
4 files changed, 40 insertions(+), 10 deletions(-)
create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
@@ -2355,7 +2356,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr
xmlopt, virConfPtr con
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid or not yet handled value '%s' "
"for VMX entry '%s' for device type '%s'"),
- fileName, fileName_name,
+ fileName ? fileName : "(not present)",
You can use NULLSTR(fileName) to get a "" in the error message.
Also, there is one more virReportError just like this below
in the FLOPPY section.
+ fileName_name,
deviceType ? deviceType : "unknown");
goto cleanup;
}
With the other virReportError touched (I don't care which way):
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
I'm good with that.
I verified other resources that use rtc... they are checking for rtc0.
Thanks, Michal
--
Julio Cesar Faracco
Em ter., 17 de mar. de 2020 às 12:32, Michal Prívozník
escreveu:
>
> On 17. 3. 2020 14:41, Julio Faracco wrote:
> > Hi Michal,
> >
> > /dev/rtc symlink is created by udev default rules.
> > Maybe we can check for both: /dev/rtc and /dev/rtc0.
>
> I think it's safe to rely on default udev behavior and just create rtc0.
> What do you think? No need to send another version, I can do the fixes
> locally before pushing.
>
> Michal
>
On Thu, Mar 12, 2020 at 02:28:22PM -0500, Eric Blake wrote:
> Creating an image that requires format probing of the backing image is
> inherently unsafe (we've had several CVEs over the years based on
> probes leaking information to the guest on a subsequent boot, although
> these days tools like libvirt are aware of the issue enough to prevent
> the worst effects). However, if our probing algorithm ever changes,
> or if other tools like libvirt determine a different probe result than
> we do, then subsequent use of that backing file under a different
> format will present corrupted data to the guest. Start a deprecation
> clock so that future qemu-img can refuse to create unsafe backing
> chains that would rely on probing. The warnings are intentionally
> emitted from the block layer rather than qemu-img (thus, all paths
> into image creation or rewriting perform the check).
>
> However, there is one time where probing is safe: if we probe raw,
> then it is safe to record that implicitly in the image (but we still
> warn, as it's better to teach the user to supply -F always than to
> make them guess when it is safe).
>
> iotest 114 specifically wants to create an unsafe image for later
> amendment rather than defaulting to our new default of recording a
> probed format, so it needs an update. While touching it, expand it to
> cover all of the various warnings enabled by this patch. iotest 290
> also shows a change to qcow messages; note that the fact that we now
> make a probed format of 'raw' explicit now results in a double
> warning, but no one should be creating new qcow images so it is not
> worth cleaning up.
>
> Signed-off-by: Eric Blake
> ---
> docs/system/deprecated.rst | 19 +++
> block.c| 21 -
> qemu-img.c | 2 +-
> tests/qemu-iotests/114 | 11 +++
> tests/qemu-iotests/114.out | 8
> tests/qemu-iotests/290.out | 5 -
> 6 files changed, 63 insertions(+), 3 deletions(-)
[A quick question ... while I'm still testing]
I just applied your v4, and I'm here:
$> git describe
v4.2.0-2399-g3cba0d19f2
Expected warning on 'create' wiht no -F:
$> ~/build/v4_tightened_qemu-img-QEMU/qemu-img create -f qcow2 -b
./base.raw ./overlay1.qcow2
qemu-img: warning: Deprecated use of backing file without explicit backing
format (detected format of raw)
Formatting './overlay1.qcow2', fmt=qcow2 size=4294967296
backing_file=./base.raw backing_fmt=raw cluster_size=65536 lazy_refcounts=off
refcount_bits=16
But here is the lack of warning with 'convert' expected?
$> ~/build/v4_tightened_qemu-img-QEMU/qemu-img convert -B ./base.raw -O
qcow2 overlay1.qcow2 flattened.qcow2
In your response on the v3, you said the above should throw a warning;
refer to Message-ID: <2fd580c2-4b94-4430-1072-ef04bbd2d...@redhat.com>
For completeness' sake:
$> ~/build/v4_tightened_qemu-img-QEMU/qemu-img info --backing-chain
flattened.qcow2
image: flattened.qcow2
file format: qcow2
virtual size: 4 GiB (4294967296 bytes)
disk size: 196 KiB
cluster_size: 65536
backing file: ./base.raw
Format specific information:
compat: 1.1
lazy refcounts: false
refcount bits: 16
corrupt: false
image: ./base.raw
file format: raw
virtual size: 4 GiB (4294967296 bytes)
disk size: 778 MiB
[...]
--
/kashyap
On 3/17/20 10:11 AM, Daniel P. Berrangé wrote:
On Tue, Mar 17, 2020 at 03:07:34PM +, Peter Maydell wrote:
On Tue, 17 Mar 2020 at 15:05, Daniel P. Berrangé wrote:
On Tue, Mar 17, 2020 at 03:00:48PM +, Peter Maydell wrote:
On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé wrote:
I don't feel like -Wno-unused-function looses anything significant, as
the GCC builds will still be reporting unused functions which will
catch majority of cases.
The most interesting difference is that clang will catch unused
static inline functions which gcc does not.
That's mostly just about dead code cruft detection IIUC. That code won't
make it into the binary if it isn't used.
Indeed, but it's nice to have the dead code cruft detection. You
can always mark the function as __attribute__((unused)) if you really
mean that it might be present but not used.
The *BSDs seem to track latest glib pretty quickly. So if we got the
unused attribute into upstream glib, we would probably have about
6-9 months before we get a build platform with the fixed glib included
where we can conditionally re-enable the unused-function warning.
Looks like glib commit https://github.com/GNOME/glib/commit/b41bff1f
(2.57.2) added G_GNUC_UNUSED to all of the functions declared during
G_DEFINE_AUTOPTR_CLEANUP_FUNC. Which version of glib is on the NetBSD
machine?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On 17. 3. 2020 14:41, Julio Faracco wrote:
> Hi Michal,
>
> /dev/rtc symlink is created by udev default rules.
> Maybe we can check for both: /dev/rtc and /dev/rtc0.
I think it's safe to rely on default udev behavior and just create rtc0.
What do you think? No need to send another version, I can do the fixes
locally before pushing.
Michal
Marc-André Lureau writes:
> Hi
>
> On Tue, Mar 17, 2020 at 12:55 PM Markus Armbruster wrote:
>>
>> This series extends QMP introspection to cover deprecation.
>> Additionally, new option -compat lets you configure what to do when
>> deprecated interfaces get used. This is intended for testing users of
>> the management interfaces. It is experimental.
>>
>> -compat deprecated-input= configures what to do when
>> deprecated input is received. Available policies:
>>
>> * accept: Accept deprecated commands and arguments (default)
>> * reject: Reject them
>> * crash: Crash
>>
>> -compat deprecated-output= configures what to do when
>> deprecated output is sent. Available output policies:
>>
>> * accept: Emit deprecated command results and events (default)
>> * hide: Suppress them
>>
>> For now, -compat covers only deprecated syntactic aspects of QMP. We
>> may want to extend it to cover semantic aspects, CLI, and experimental
>> features.
>
> I suggest to use a qmp- prefix for qmp-related policies.
The interface is general.
The implemented infrastructure is QAPI-only.
Its application is QMP-only.
If our CLI was QAPIfied, I'd certainly apply it there, too. I intend to
resume exploring CLI QAPIfication "real soon now".
Not covering CLI now is a bit like not covering semantic aspects of QMP.
Imagine the thing covered CLI. Would we want to have separate -compat
deprecated-qmp-input, deprecated-cli-input? I doubt it. I think we
want a single policy for all host interfaces.
Imagine it covered deprecated semantic aspects of QMP. Would we want to
have a separate flag for that? Again, I doubt it.
For what it's worth, the interface is documented as experimental.
On Tue, Mar 17, 2020 at 04:05:12PM +0100, Peter Krempa wrote:
> String list was allocated without extra element.
>
> Peter Krempa (3):
> qemuBlockBitmapsHandleCommitStart: Fix allocation of string list
> qemuBlockBitmapsHandleCommitFinish: Use proper variable to iterate
> qemublocktest: Add tests for re-enabling of bitmaps after commit
Reviewed-by: Daniel P. Berrangé
Tested-by: Daniel P. Berrangé
this fixes the freebsd crashes our CI saw.
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 :|
On a Tuesday in 2020, Peter Krempa wrote:
String list was allocated without extra element.
Peter Krempa (3):
qemuBlockBitmapsHandleCommitStart: Fix allocation of string list
qemuBlockBitmapsHandleCommitFinish: Use proper variable to iterate
qemublocktest: Add tests for re-enabling of bitmaps after commit
src/qemu/qemu_block.c | 4 +-
.../bitmap/snapshots-synthetic-broken.json| 18
.../bitmap/snapshots-synthetic-broken.out | 2 +
.../snapshots-synthetic-broken-1-2| 30
.../snapshots-synthetic-broken-1-3| 46 +++
.../snapshots-synthetic-broken-1-4| 46 +++
.../snapshots-synthetic-broken-1-5| 46 +++
.../snapshots-synthetic-broken-2-3| 46 +++
.../snapshots-synthetic-broken-2-4| 46 +++
.../snapshots-synthetic-broken-2-5| 46 +++
10 files changed, 328 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
On a Tuesday in 2020, Peter Krempa wrote:
The function repeatedly checked the first element rather than interating
iterating
through the array.
Reported-by: Daniel P. Berrangé
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Jano
signature.asc
Description: PGP signature
On Tue, Mar 17, 2020 at 03:07:34PM +, Peter Maydell wrote:
> On Tue, 17 Mar 2020 at 15:05, Daniel P. Berrangé wrote:
> >
> > On Tue, Mar 17, 2020 at 03:00:48PM +, Peter Maydell wrote:
> > > On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé
> > > wrote:
> > > > I don't feel like -Wno-unused-function looses anything significant, as
> > > > the GCC builds will still be reporting unused functions which will
> > > > catch majority of cases.
> > >
> > > The most interesting difference is that clang will catch unused
> > > static inline functions which gcc does not.
> >
> > That's mostly just about dead code cruft detection IIUC. That code won't
> > make it into the binary if it isn't used.
>
> Indeed, but it's nice to have the dead code cruft detection. You
> can always mark the function as __attribute__((unused)) if you really
> mean that it might be present but not used.
The *BSDs seem to track latest glib pretty quickly. So if we got the
unused attribute into upstream glib, we would probably have about
6-9 months before we get a build platform with the fixed glib included
where we can conditionally re-enable the unused-function warning.
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 :|
On Tue, 17 Mar 2020 at 15:05, Daniel P. Berrangé wrote:
>
> On Tue, Mar 17, 2020 at 03:00:48PM +, Peter Maydell wrote:
> > On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé
> > wrote:
> > > I don't feel like -Wno-unused-function looses anything significant, as
> > > the GCC builds will still be reporting unused functions which will
> > > catch majority of cases.
> >
> > The most interesting difference is that clang will catch unused
> > static inline functions which gcc does not.
>
> That's mostly just about dead code cruft detection IIUC. That code won't
> make it into the binary if it isn't used.
Indeed, but it's nice to have the dead code cruft detection. You
can always mark the function as __attribute__((unused)) if you really
mean that it might be present but not used.
thanks
-- PMM
On Tue, Mar 17, 2020 at 03:00:48PM +, Peter Maydell wrote:
> On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé wrote:
> > I don't feel like -Wno-unused-function looses anything significant, as
> > the GCC builds will still be reporting unused functions which will
> > catch majority of cases.
>
> The most interesting difference is that clang will catch unused
> static inline functions which gcc does not.
That's mostly just about dead code cruft detection IIUC. That code won't
make it into the binary if it isn't used.
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 :|
On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé wrote:
> I don't feel like -Wno-unused-function looses anything significant, as
> the GCC builds will still be reporting unused functions which will
> catch majority of cases.
The most interesting difference is that clang will catch unused
static inline functions which gcc does not.
thanks
-- PMM
On Tue, Mar 17, 2020 at 09:40:00AM -0500, Eric Blake wrote:
> On 3/17/20 9:00 AM, Peter Maydell wrote:
> > On Tue, 17 Mar 2020 at 04:38, John Snow wrote:
>
> > >
> > >block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12
> > > 16:36:46 -0400)
> > >
> > >
> > > Pull request
> > >
> > > ---
> >
> > Hi; this fails to compile with clang:
> >
> > /home/petmay01/linaro/qemu-for-merges/nbd/server.c:1937:1: error:
> > unused function 'glib_listautoptr_cleanup_NBDExtentArray'
> > [-Werror,-Wunused-function]
> > G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
> > ^
> > /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro
> > 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
> >static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList
> > **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \
> > ^
> > /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro
> > '_GLIB_AUTOPTR_LIST_FUNC_NAME'
> > #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName)
> > glib_listautoptr_cleanup_##TypeName
> > ^
> > :49:1: note: expanded from here
> > glib_listautoptr_cleanup_NBDExtentArray
> > ^
>
> Should we add -Wno-unused-function to CFLAGS when dealing with a version of
> clang that complains about that version of glib's headers? Is it fixed in a
> newer version of glib, where we could just backport a newer definition of
> G_DEFINE_AUTOPTR_CLEANUP_FUNC() that adds whatever annotations are needed to
> shut the compiler up?
>
> On IRC, danpb pointed me to libvirt's solution:
> https://libvirt.org/git/?p=libvirt.git;a=commit;h=44e7f029
>
> Maybe we just write our own macro wrapper around
> G_DEFINE_AUTOPTR_CLEANUP_FUNC which takes care of adding necessary
> annotations and use that instead (and our macro name might be shorter...)
My preference is to stick with regular glib functions/macros whereever
practical, rather than inventing QEMU replacements which add a knowledge
burden for contributors. That's why we moved to -Wno-unused-function in
libvirt.
I don't feel like -Wno-unused-function looses anything significant, as
the GCC builds will still be reporting unused functions which will
catch majority of cases.
Possibly we could figure out a patch for glib upstream that uses pragma
push/pop to squelch the warning ? They are quite receptive to patches
IME.
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 :|
On 3/17/20 9:00 AM, Peter Maydell wrote:
On Tue, 17 Mar 2020 at 04:38, John Snow wrote:
block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46
-0400)
Pull request
---
Hi; this fails to compile with clang:
/home/petmay01/linaro/qemu-for-merges/nbd/server.c:1937:1: error:
unused function 'glib_listautoptr_cleanup_NBDExtentArray'
[-Werror,-Wunused-function]
G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
^
/usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro
'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList
**_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \
^
/usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro
'_GLIB_AUTOPTR_LIST_FUNC_NAME'
#define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName)
glib_listautoptr_cleanup_##TypeName
^
:49:1: note: expanded from here
glib_listautoptr_cleanup_NBDExtentArray
^
Should we add -Wno-unused-function to CFLAGS when dealing with a version
of clang that complains about that version of glib's headers? Is it
fixed in a newer version of glib, where we could just backport a newer
definition of G_DEFINE_AUTOPTR_CLEANUP_FUNC() that adds whatever
annotations are needed to shut the compiler up?
On IRC, danpb pointed me to libvirt's solution:
https://libvirt.org/git/?p=libvirt.git;a=commit;h=44e7f029
Maybe we just write our own macro wrapper around
G_DEFINE_AUTOPTR_CLEANUP_FUNC which takes care of adding necessary
annotations and use that instead (and our macro name might be shorter...)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On Tue, 17 Mar 2020 at 04:38, John Snow wrote:
>
> The following changes since commit 6e8a73e911f066527e775e04b98f31ebd19db600:
>
> Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request'
> into staging (2020-03-11 14:41:27 +)
>
> are available in the Git repository at:
>
> https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
>
> for you to fetch changes up to 34b456d485a4df3a88116fb5ef0c418f2f12990d:
>
> block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46
> -0400)
>
>
> Pull request
>
> ---
Hi; this fails to compile with clang:
/home/petmay01/linaro/qemu-for-merges/nbd/server.c:1937:1: error:
unused function 'glib_listautoptr_cleanup_NBDExtentArray'
[-Werror,-Wunused-function]
G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
^
/usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro
'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList
**_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \
^
/usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro
'_GLIB_AUTOPTR_LIST_FUNC_NAME'
#define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName)
glib_listautoptr_cleanup_##TypeName
^
:49:1: note: expanded from here
glib_listautoptr_cleanup_NBDExtentArray
^
thanks
-- PMM
Hi Michal,
/dev/rtc symlink is created by udev default rules.
Maybe we can check for both: /dev/rtc and /dev/rtc0.
--
Julio Cesar Faracco
Em seg., 16 de mar. de 2020 às 15:16, Michal Prívozník
escreveu:
>
> On 2. 3. 2020 1:54, Julio Faracco wrote:
> > This commit share host Real Time Clock device (rtc) into LXC containers
> > to support hardware clock. This should be available setting up a `rtc`
> > timer under clock section. Since this option is not emulated, it should
> > be available only for `localtime` clock. This option should be readonly
> > due to security reasons.
> >
> > Before:
> > root# hwclock --verbose
> > hwclock from util-linux 2.32.1
> > System Time: 1581877557.598365
> > Trying to open: /dev/rtc0
> > Trying to open: /dev/rtc
> > Trying to open: /dev/misc/rtc
> > No usable clock interface found.
> > hwclock: Cannot access the Hardware Clock via any known method.
> >
> > Now:
> > root# hwclock
> > 2020-02-16 18:23:55.374134+00:00
> > root# hwclock -w
> > hwclock: ioctl(RTC_SET_TIME) to /dev/rtc to set the time failed:
> > Permission denied
> >
> > Signed-off-by: Julio Faracco
> > ---
> > docs/formatdomain.html.in | 2 +-
> > src/lxc/lxc_cgroup.c | 33
> > src/lxc/lxc_controller.c | 66 +++
> > 3 files changed, 100 insertions(+), 1 deletion(-)
>
> On my system, the /dev/rtc is just a symlink to /dev/rtc0. Should we
> create the symlink too or is it okay to just create /dev/rtc?
>
> Otherwise looking good.
>
> Michal
>
Hi
On Tue, Mar 17, 2020 at 12:55 PM Markus Armbruster wrote:
>
> This series extends QMP introspection to cover deprecation.
> Additionally, new option -compat lets you configure what to do when
> deprecated interfaces get used. This is intended for testing users of
> the management interfaces. It is experimental.
>
> -compat deprecated-input= configures what to do when
> deprecated input is received. Available policies:
>
> * accept: Accept deprecated commands and arguments (default)
> * reject: Reject them
> * crash: Crash
>
> -compat deprecated-output= configures what to do when
> deprecated output is sent. Available output policies:
>
> * accept: Emit deprecated command results and events (default)
> * hide: Suppress them
>
> For now, -compat covers only deprecated syntactic aspects of QMP. We
> may want to extend it to cover semantic aspects, CLI, and experimental
> features.
I suggest to use a qmp- prefix for qmp-related policies.
> PATCH 01-04: Documentation fixes
> PATCH 05-10: Test improvements
> PATCH 11-24: Add feature flags to remaining user-defined types and to
> struct members
> PATCH 25-26: New special feature 'deprecated', visible in
> introspection
> PATCH 27-34: New -compat to set policy for handling stuff marked with
> feature 'deprecated'
>
> v4:
> PATCH 05+07: Temporary memory leak plugged [Marc-André]
> PATCH 23: Rewritten [Marc-André]
> PATCH 24: Comment typo [Marc-André]
> PATCH 30: Memory leaks plugged
>
> v3:
> * Rebased, non-trivial conflicts in PATCH 01+26+27+34 due to RST
> conversion and code motion
> * PATCH 28-29: Old PATCH 28 split up to ease review
> * PATCH 30-31: New
> * PATCH 32-33: Old PATCH 29 split up to ease review
>
> Comparison to RFC (24 Oct 2019):
> * Cover arguments and results in addition to commands and events
> * Half-baked "[RFC PATCH 18/19] qapi: Include a warning in the
> response to a deprecated command" dropped
>
> See also last item of
> Subject: Minutes of KVM Forum BoF on deprecating stuff
> Date: Fri, 26 Oct 2018 16:03:51 +0200
> Message-ID: <87mur0ls8o@dusky.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html
>
> Cc: Lukáš Doktor
> Cc: libgues...@redhat.com
> Cc: libvir-list@redhat.com
> Cc: Daniel P. Berrange
> Cc: Peter Krempa
> Cc: Kevin Wolf
>
> Markus Armbruster (34):
> qemu-doc: Belatedly document QMP command arg & result deprecation
> qapi: Belatedly update doc comment for @wait deprecation
> docs/devel/qapi-code-gen: Clarify allow-oob introspection
> docs/devel/qapi-code-gen: Document 'features' introspection
> tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
> tests/test-qmp-cmds: Check responses more thoroughly
> tests/test-qmp-cmds: Simplify test data setup
> tests/test-qmp-event: Simplify test data setup
> tests/test-qmp-event: Use qobject_is_equal()
> tests/test-qmp-event: Check event is actually emitted
> qapi/schema: Clean up around QAPISchemaEntity.connect_doc()
> qapi: Add feature flags to remaining definitions
> qapi: Consistently put @features parameter right after @ifcond
> qapi/introspect: Rename *qlit* to reduce confusion
> qapi/introspect: Factor out _make_tree()
> qapi/schema: Change _make_features() to a take feature list
> qapi/schema: Reorder classes so related ones are together
> qapi/schema: Rename QAPISchemaObjectType{Variant,Variants}
> qapi/schema: Call QAPIDoc.connect_member() in just one place
> qapi: Add feature flags to struct members
> qapi: Inline do_qmp_dispatch() into qmp_dispatch()
> qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP
> qapi: Simplify how qmp_dispatch() gets the request ID
> qapi: Replace qmp_dispatch()'s TODO comment by an explanation
> qapi: New special feature flag "deprecated"
> qapi: Mark deprecated QMP parts with feature 'deprecated'
> qemu-options: New -compat to set policy for deprecated interfaces
> qapi: Implement deprecated-output=hide for QMP command results
> qapi: Implement deprecated-output=hide for QMP events
> qapi: Implement deprecated-output=hide for QMP event data
> qapi: Implement deprecated-output=hide for QMP introspection
> qapi: Implement deprecated-input=reject for QMP commands
> qapi: Implement deprecated-input=reject for QMP command arguments
> qapi: New -compat deprecated-input=crash
>
> docs/devel/qapi-code-gen.txt | 79 ++-
> docs/system/deprecated.rst| 48 +-
> tests/qapi-schema/doc-good.texi | 32 ++
> qapi/block-core.json | 48 +-
> qapi/block.json | 30 +-
> qapi/char.json| 1 +
> qapi/compat.json | 52 ++
> qapi/control.json | 11 +-
> qapi/introspect.json | 28 +-
> qapi/machine.json | 34 +-
> qapi/
On Tue, Mar 17, 2020 at 12:06:45PM +0100, Christophe de Dinechin wrote:
> +1 on the initial thread b
>
> > On 6 Mar 2020, at 15:54, Daniel Henrique Barboza
> > wrote:
> >
> >
> >
> > On 3/6/20 8:44 AM, Daniel P. Berrangé wrote:
> >
> >
> > [...]
> >
> >
> > What happens with this mailing list when the migration to the new workflow
> > is
> > completed with all the repos? Is it still going to be used for discussions,
> > questions, RFCs and etcetera? I'd rather be in Gitlab watching opened issues
> > and merge requests all the time, without the need to check the Libvirt ML
> > ever again.
> >
> > And apparently we're leaning towards Gitlab. I'll not be standing here
> > defending closed-source, Microsoft based Github, but I'm curious: aside from
> > that (and that reason alone is enough, no need to grab the pitchforks),
> > is there any other technical advantage for going Gitlab? I suppose most
> > existing "coding support tools" are Github friendly already. Also, due to
> > Microsoft deep pockets, Github will probably experience less downtime and
> > have a
> > better support overall in case something goes wrong.
>
> GitHub and GitLab have different approaches to CI, with pros and cons on
> both sides. Obviously, it is easier to get stuff tested on Windows with
> GitHub,
> for example.
>
> You can use both, with automatic mirroring of the commits. For some of
> my own projects, I have dual push targets in git (triple, actually,
> SourceForge),
> and then I get two sets of (different) CI tests on a push. For example,
> GitLab will test a number of Linux targets like Ubuntu, etc, while
> GitHub will test macOS and someday Windows.
NB, windows testing is covered by mingw64 cross compilers, at least for
build testing. If we wanted to run unit tests we can use wine, but it
hasn't been a priority. For real functional tests we'd need a real
windows install, but that's even further down the list of things we
care about.
macOS testing we get via Travis and that's the main gap we have with
GitLab, unless we get access some hardware we can setup as a GitLab
custom CI runner.
> I am not aware of a good way to sync issues, though, only commits.
> Anybody knows differently?
Syncing issues & merge requests is tricky to do accurately, because you
need to parse the comments to identify references to usernames and
issues, etc. IMHO it just isn't worth the hassle to. Syncing everything
would also make google search results and split attention with people
not being sure which is the master vs mirror. We already have that
problem to some extent with the existing commit mirroring, so I'm
loathe to make it more confusing.
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 :|
This series extends QMP introspection to cover deprecation.
Additionally, new option -compat lets you configure what to do when
deprecated interfaces get used. This is intended for testing users of
the management interfaces. It is experimental.
-compat deprecated-input= configures what to do when
deprecated input is received. Available policies:
* accept: Accept deprecated commands and arguments (default)
* reject: Reject them
* crash: Crash
-compat deprecated-output= configures what to do when
deprecated output is sent. Available output policies:
* accept: Emit deprecated command results and events (default)
* hide: Suppress them
For now, -compat covers only deprecated syntactic aspects of QMP. We
may want to extend it to cover semantic aspects, CLI, and experimental
features.
PATCH 01-04: Documentation fixes
PATCH 05-10: Test improvements
PATCH 11-24: Add feature flags to remaining user-defined types and to
struct members
PATCH 25-26: New special feature 'deprecated', visible in
introspection
PATCH 27-34: New -compat to set policy for handling stuff marked with
feature 'deprecated'
v4:
PATCH 05+07: Temporary memory leak plugged [Marc-André]
PATCH 23: Rewritten [Marc-André]
PATCH 24: Comment typo [Marc-André]
PATCH 30: Memory leaks plugged
v3:
* Rebased, non-trivial conflicts in PATCH 01+26+27+34 due to RST
conversion and code motion
* PATCH 28-29: Old PATCH 28 split up to ease review
* PATCH 30-31: New
* PATCH 32-33: Old PATCH 29 split up to ease review
Comparison to RFC (24 Oct 2019):
* Cover arguments and results in addition to commands and events
* Half-baked "[RFC PATCH 18/19] qapi: Include a warning in the
response to a deprecated command" dropped
See also last item of
Subject: Minutes of KVM Forum BoF on deprecating stuff
Date: Fri, 26 Oct 2018 16:03:51 +0200
Message-ID: <87mur0ls8o@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html
Cc: Lukáš Doktor
Cc: libgues...@redhat.com
Cc: libvir-list@redhat.com
Cc: Daniel P. Berrange
Cc: Peter Krempa
Cc: Kevin Wolf
Markus Armbruster (34):
qemu-doc: Belatedly document QMP command arg & result deprecation
qapi: Belatedly update doc comment for @wait deprecation
docs/devel/qapi-code-gen: Clarify allow-oob introspection
docs/devel/qapi-code-gen: Document 'features' introspection
tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
tests/test-qmp-cmds: Check responses more thoroughly
tests/test-qmp-cmds: Simplify test data setup
tests/test-qmp-event: Simplify test data setup
tests/test-qmp-event: Use qobject_is_equal()
tests/test-qmp-event: Check event is actually emitted
qapi/schema: Clean up around QAPISchemaEntity.connect_doc()
qapi: Add feature flags to remaining definitions
qapi: Consistently put @features parameter right after @ifcond
qapi/introspect: Rename *qlit* to reduce confusion
qapi/introspect: Factor out _make_tree()
qapi/schema: Change _make_features() to a take feature list
qapi/schema: Reorder classes so related ones are together
qapi/schema: Rename QAPISchemaObjectType{Variant,Variants}
qapi/schema: Call QAPIDoc.connect_member() in just one place
qapi: Add feature flags to struct members
qapi: Inline do_qmp_dispatch() into qmp_dispatch()
qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP
qapi: Simplify how qmp_dispatch() gets the request ID
qapi: Replace qmp_dispatch()'s TODO comment by an explanation
qapi: New special feature flag "deprecated"
qapi: Mark deprecated QMP parts with feature 'deprecated'
qemu-options: New -compat to set policy for deprecated interfaces
qapi: Implement deprecated-output=hide for QMP command results
qapi: Implement deprecated-output=hide for QMP events
qapi: Implement deprecated-output=hide for QMP event data
qapi: Implement deprecated-output=hide for QMP introspection
qapi: Implement deprecated-input=reject for QMP commands
qapi: Implement deprecated-input=reject for QMP command arguments
qapi: New -compat deprecated-input=crash
docs/devel/qapi-code-gen.txt | 79 ++-
docs/system/deprecated.rst| 48 +-
tests/qapi-schema/doc-good.texi | 32 ++
qapi/block-core.json | 48 +-
qapi/block.json | 30 +-
qapi/char.json| 1 +
qapi/compat.json | 52 ++
qapi/control.json | 11 +-
qapi/introspect.json | 28 +-
qapi/machine.json | 34 +-
qapi/migration.json | 36 +-
qapi/misc.json| 13 +-
qapi/qapi-schema.json | 1 +
include/qapi/compat-policy.h | 20 +
include/qapi/qmp/dispatch.h | 1 +
include/qapi/qobject-input-visitor.h | 9 +
i
+1 on the initial thread b
> On 6 Mar 2020, at 15:54, Daniel Henrique Barboza
> wrote:
>
>
>
> On 3/6/20 8:44 AM, Daniel P. Berrangé wrote:
>
>
> [...]
>
>
> What happens with this mailing list when the migration to the new workflow is
> completed with all the repos? Is it still going to be used for discussions,
> questions, RFCs and etcetera? I'd rather be in Gitlab watching opened issues
> and merge requests all the time, without the need to check the Libvirt ML
> ever again.
>
> And apparently we're leaning towards Gitlab. I'll not be standing here
> defending closed-source, Microsoft based Github, but I'm curious: aside from
> that (and that reason alone is enough, no need to grab the pitchforks),
> is there any other technical advantage for going Gitlab? I suppose most
> existing "coding support tools" are Github friendly already. Also, due to
> Microsoft deep pockets, Github will probably experience less downtime and
> have a
> better support overall in case something goes wrong.
GitHub and GitLab have different approaches to CI, with pros and cons on
both sides. Obviously, it is easier to get stuff tested on Windows with GitHub,
for example.
You can use both, with automatic mirroring of the commits. For some of
my own projects, I have dual push targets in git (triple, actually,
SourceForge),
and then I get two sets of (different) CI tests on a push. For example,
GitLab will test a number of Linux targets like Ubuntu, etc, while
GitHub will test macOS and someday Windows.
I am not aware of a good way to sync issues, though, only commits.
Anybody knows differently?
>
>
> Thanks,
>
>
> DHB
>
>
On Tue, Mar 17, 2020 at 11:12:54AM +0100, Peter Krempa wrote:
> On Tue, Mar 17, 2020 at 09:47:22 +, Daniel Berrange wrote:
> > On Tue, Mar 17, 2020 at 09:26:44AM +0100, Peter Krempa wrote:
> > > On Tue, Mar 17, 2020 at 09:20:07 +0100, Michal Privoznik wrote:
> > > > On 17. 3. 2020 8:52, Peter Krempa wrote:
> > > > > On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
> > > > >> We've discussed the idea of replacing our mailing list review
> > > > >> workflow with
> > > > >> a merge request workflow in various places, over the last 6 months
> > > > >> or so,
> > > > >> but never made a concrete decision.
> > > > >
> > > > > One other thing that worries me about this is that we've finally
> > > > > established a way close to qemu developers for notifying us if they
> > > > > are
> > > > > going to deprecate something or change something important.
> > > > >
> > > > > With moving development to some random web page with non-standard
> > > > > interfaces this will just mean that the notifications in this process
> > > > > will either stay on the old mailing list or be forgotten if we don't
> > > > > act
> > > > > on them.
> > > > >
> > > > > Moving development to some other place will in this regard just mean
> > > > > that we'll have to watch two places at the same time.
> > > > >
> > > > > While this seems to be a very low impact thing, the advantages of the
> > > > > new process you've outlined will only ever apply to drive-by
> > > > > contributors. Anybody wanting to take it seriously will necessarily
> > > > > need
> > > > > to subscribe to the mailing list anyways.
> > > > >
> > > > > In the end I just don't want to destroy the relationship with qemu
> > > > > developers by not acting on the notifications of change they send to
> > > > > us.
> > > > >
> > > > >
> > > >
> > > > I don't think I share this view. The way qemu developers notify us is
> > > > cross-posting to libvir-list. They can still do that and with the
> > > > traffic on the list going down it will be pretty easy to spot these
> > > > cross posts. Or am I missing something?
> > >
> > > Yes. As mentioned above though you need to be subscribed to the list
> > > though. Also as mentioned above, that means that any serious developer
> > > will need to be subscribe to the list. So all the point of not having to
> > > subscribe to the list applies only to drive-by contributors.
> >
> > Our long term contributors are the only ones who are likely to take any
> > actions based on the QEMU cross-posted messages, so I don't think we'll
> > loose anything measurable in this regard.
>
> Sounds a bit hypocritical then. The merge request workflow is
> inconveniencing long term contributors so that we can attract new
> contributors. If the new contributors aren't seeing those they won't be
> able to act on them. And if we don't expect new contributors to be
> involved to that extent, why are we even doing this?
It isn't hypocritical, it is pragmatic approach to the interaction
we have.
By turning deprecations into issues they become visible to all
developers.
> > We could probably do with formalizing our handling of QEMU deprecations
> > too. There have been a couple of occassions where we saw the message buta
>
> I think that requiring qemu developers to sign up to some web-page to
> file something are going to be detremental to the effort. The
> notifications are based mostly on the fact that changes to the file
> documenting the deprecations are automatically CCd to libvirt-list.
I'm not suggesting QEMU devs file the issues, I'm saying that when
libvirt is copied on a deprecation alert, we should file a issue
against libvirt ourselves to track it (assuming it affects libvirt)
> > then failed to take action. It might be worth us explicitly filing an
> > issue against libvirt for every deprecation warning, so that we know
> > what is outtstanding on our todo list in this regard.
>
> There are also instances, where we've seen the message, filed bugs
> (issues) and not taken action. It's always human factor.
>
> There are at least these that I know of and there wasn't any action
> taken:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1783355
> https://bugzilla.redhat.com/show_bug.cgi?id=1717899
>
> I didn't even bother looking into the upstream bugzilla btw. There's a
> giant pile of stuff which was filed but nobody cares about. I'm not sure
> if we want to mirror that to whatever-hub/lab but either way given some
> time I feel it will end up the same way as the upstream bugzilla.
We need todo better at bug triage in general - ignoring stuff just
makes the problem worse.
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 :|
On Tue, Mar 17, 2020 at 09:47:22 +, Daniel Berrange wrote:
> On Tue, Mar 17, 2020 at 09:26:44AM +0100, Peter Krempa wrote:
> > On Tue, Mar 17, 2020 at 09:20:07 +0100, Michal Privoznik wrote:
> > > On 17. 3. 2020 8:52, Peter Krempa wrote:
> > > > On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
> > > >> We've discussed the idea of replacing our mailing list review workflow
> > > >> with
> > > >> a merge request workflow in various places, over the last 6 months or
> > > >> so,
> > > >> but never made a concrete decision.
> > > >
> > > > One other thing that worries me about this is that we've finally
> > > > established a way close to qemu developers for notifying us if they are
> > > > going to deprecate something or change something important.
> > > >
> > > > With moving development to some random web page with non-standard
> > > > interfaces this will just mean that the notifications in this process
> > > > will either stay on the old mailing list or be forgotten if we don't act
> > > > on them.
> > > >
> > > > Moving development to some other place will in this regard just mean
> > > > that we'll have to watch two places at the same time.
> > > >
> > > > While this seems to be a very low impact thing, the advantages of the
> > > > new process you've outlined will only ever apply to drive-by
> > > > contributors. Anybody wanting to take it seriously will necessarily need
> > > > to subscribe to the mailing list anyways.
> > > >
> > > > In the end I just don't want to destroy the relationship with qemu
> > > > developers by not acting on the notifications of change they send to us.
> > > >
> > > >
> > >
> > > I don't think I share this view. The way qemu developers notify us is
> > > cross-posting to libvir-list. They can still do that and with the
> > > traffic on the list going down it will be pretty easy to spot these
> > > cross posts. Or am I missing something?
> >
> > Yes. As mentioned above though you need to be subscribed to the list
> > though. Also as mentioned above, that means that any serious developer
> > will need to be subscribe to the list. So all the point of not having to
> > subscribe to the list applies only to drive-by contributors.
>
> Our long term contributors are the only ones who are likely to take any
> actions based on the QEMU cross-posted messages, so I don't think we'll
> loose anything measurable in this regard.
Sounds a bit hypocritical then. The merge request workflow is
inconveniencing long term contributors so that we can attract new
contributors. If the new contributors aren't seeing those they won't be
able to act on them. And if we don't expect new contributors to be
involved to that extent, why are we even doing this?
> We could probably do with formalizing our handling of QEMU deprecations
> too. There have been a couple of occassions where we saw the message buta
I think that requiring qemu developers to sign up to some web-page to
file something are going to be detremental to the effort. The
notifications are based mostly on the fact that changes to the file
documenting the deprecations are automatically CCd to libvirt-list.
This way if a drive-by qemu contributor sends patches they might not
care enough to go through the notification process in the first place.
> then failed to take action. It might be worth us explicitly filing an
> issue against libvirt for every deprecation warning, so that we know
> what is outtstanding on our todo list in this regard.
There are also instances, where we've seen the message, filed bugs
(issues) and not taken action. It's always human factor.
There are at least these that I know of and there wasn't any action
taken:
https://bugzilla.redhat.com/show_bug.cgi?id=1783355
https://bugzilla.redhat.com/show_bug.cgi?id=1717899
I didn't even bother looking into the upstream bugzilla btw. There's a
giant pile of stuff which was filed but nobody cares about. I'm not sure
if we want to mirror that to whatever-hub/lab but either way given some
time I feel it will end up the same way as the upstream bugzilla.
On Tue, Mar 17, 2020 at 10:01:24AM +0100, Peter Krempa wrote:
> On Tue, Mar 17, 2020 at 09:42:30 +0100, Michal Privoznik wrote:
> > On 17. 3. 2020 9:26, Peter Krempa wrote:
> > > On Tue, Mar 17, 2020 at 09:20:07 +0100, Michal Privoznik wrote:
> > >> On 17. 3. 2020 8:52, Peter Krempa wrote:
>
> [...]
>
> > >> I don't think I share this view. The way qemu developers notify us is
> > >> cross-posting to libvir-list. They can still do that and with the
> > >> traffic on the list going down it will be pretty easy to spot these
> > >> cross posts. Or am I missing something?
> > >
> > > Yes. As mentioned above though you need to be subscribed to the list
> > > though. Also as mentioned above, that means that any serious developer
> > > will need to be subscribe to the list. So all the point of not having to
> > > subscribe to the list applies only to drive-by contributors.
> > >
> >
> > Sure, but I thought this is expected. Every project has a place to
> > discuss ideas, make decisions. Some use pull requests to do that (please
> > don't), some have a mailing list. I see libvirt in the latter group. And
> > to some extent, we are already in that situation. I mean, if I were a
> > drive by contributor, I would subscribe to the list, post my patches and
> > ignore the rest of incoming e-mails from the list.
> >
> > Maybe I'm misunderstanding and what is suggested it to move even all the
> > discussion to gitlab. If that is the case, I stand by you. We should not
> > do that. But if we are moving just the repo then I guess it is fine.
>
> See the second paragraph of another subthread of this discussion:
>
> https://www.redhat.com/archives/libvir-list/2020-March/msg00196.html
>
> I wanted to raise this issue separately so that it's not burried. At any
> rate, the idea of switching to the forge is to stop using email.
>
> While this might appeal to upper layer developers where the cool new
> deveopment approaches are used, we should not jeopardize our
> relationship with qemu as we are trying to offer them our value
> especially in the region of deprecation and changes of interfaces which
> we are making transparent to users.
>
> And if we are not going to kill off the libvir-list, the wins described
> in the original thread are not as clear cut, as it just adds things you
> have to watch/deal with, but for any real work, you'll have to endure
> the pain from both email and web-based forges.
Let me quote the paragraph in question:
> Based on what I see with other projects, they generally encourage design
> discussions to happen in the issue tracker. The designs may lead to many
> distinct pieces of work, each of which get turned into todo items and
> associated with separate merge requests. So the issue tracker is basically
> used to discuss / plan / coordinate all the non-coding work. With this
> approach there's not really much compelling reason to keep using the
> libvir-list. If you look at systemd, after they moved to merge requests,
> their mailing list still exists, but it mostly just has end user questions.
> For libvirt this role is filled by the libvirt-users mailing list.
> I wouldn't shut down libvir-list, but I wouldn't expect significant
> traffic in the long term.
If I understand it correctly there is no plan to kill libvir-list.
I completely support your concern about collaboration with QEMU
developers but I don't thing this move will affect that in any way.
Yes, existing core developers would probably have to watch two places
for libvirt development and yes new core developers would have to
subscribe to libvir-list as well if they need to cooperate with QEMU,
but I don't thing the real impact on existing core developers is that
significant.
If we move to GitLab I guess that most of us will configure email based
notifications for new pull requests or issues and since it's an email
you can do whatever you like with it, for example filter it into the
same folder as libvir-list.
Pavel
signature.asc
Description: PGP signature
On Fri, Mar 13, 2020 at 09:45:15 +0100, Christian Ehrhardt wrote:
> On Tue, Mar 10, 2020 at 11:48 AM Christian Ehrhardt <
> christian.ehrha...@canonical.com> wrote:
>
> > One of the mitigation methods for TAA[1] is to disable TSX
> > support on the host system. Linux added a mechanism to disable
> > TSX globally through the kernel command line, and many Linux
> > distributions now default to tsx=off. This makes existing CPU
> > models that have HLE and RTM enabled not usable anymore.
> >
> > Add new versions of all CPU models that have the HLE and RTM
> > features enabled, that can be used when TSX is disabled in the
> > host system.
> >
> > On systems disabling the features without those types defined
> > in cpu-maps users end up without modern CPU types in the list
> > of usable CPUs to use in the likes of virsh domcapabilities
> > or tools higher in the stack like virt-manager.
> >
> > This adds:
> > -Cascadelake-Server-noTSX
> > -Icelake-Client-noTSX
> > -Icelake-Server-noTSX
> > -Skylake-Server-noTSX-IBRS
> > -Skylake-Client-noTSX-IBRS
> >
> > Introduced in QEMU by commit v4.2.0-rc2-3-g9ab2237f19 (function)
> > and commit v4.2.0-rc2-4-g02fa60d101 (names)
> >
>
> Ping - anything else that we need for this v2 right now to accept it?
>
> I know in the long run Jiri would want to auto-select the non noTSX types.
> But as I outlined before I'd consider this a later change or would need
> some guidance where/how it is envisioned to do tat preference.
Sorry, I already started working on this additional patch as I'd like it
to be pushed right after yours. But I got distracted by other higher
priority things. I hope to finish it and send for a review soon.
Jirka
On Tue, Mar 17, 2020 at 09:26:44AM +0100, Peter Krempa wrote:
> On Tue, Mar 17, 2020 at 09:20:07 +0100, Michal Privoznik wrote:
> > On 17. 3. 2020 8:52, Peter Krempa wrote:
> > > On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
> > >> We've discussed the idea of replacing our mailing list review workflow
> > >> with
> > >> a merge request workflow in various places, over the last 6 months or so,
> > >> but never made a concrete decision.
> > >
> > > One other thing that worries me about this is that we've finally
> > > established a way close to qemu developers for notifying us if they are
> > > going to deprecate something or change something important.
> > >
> > > With moving development to some random web page with non-standard
> > > interfaces this will just mean that the notifications in this process
> > > will either stay on the old mailing list or be forgotten if we don't act
> > > on them.
> > >
> > > Moving development to some other place will in this regard just mean
> > > that we'll have to watch two places at the same time.
> > >
> > > While this seems to be a very low impact thing, the advantages of the
> > > new process you've outlined will only ever apply to drive-by
> > > contributors. Anybody wanting to take it seriously will necessarily need
> > > to subscribe to the mailing list anyways.
> > >
> > > In the end I just don't want to destroy the relationship with qemu
> > > developers by not acting on the notifications of change they send to us.
> > >
> > >
> >
> > I don't think I share this view. The way qemu developers notify us is
> > cross-posting to libvir-list. They can still do that and with the
> > traffic on the list going down it will be pretty easy to spot these
> > cross posts. Or am I missing something?
>
> Yes. As mentioned above though you need to be subscribed to the list
> though. Also as mentioned above, that means that any serious developer
> will need to be subscribe to the list. So all the point of not having to
> subscribe to the list applies only to drive-by contributors.
Our long term contributors are the only ones who are likely to take any
actions based on the QEMU cross-posted messages, so I don't think we'll
loose anything measurable in this regard.
We could probably do with formalizing our handling of QEMU deprecations
too. There have been a couple of occassions where we saw the message but
then failed to take action. It might be worth us explicitly filing an
issue against libvirt for every deprecation warning, so that we know
what is outtstanding on our todo list in this regard.
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 :|
On 17. 3. 2020 1:01, Daniel Henrique Barboza wrote:
> Commit 7b79ee2f78 makes assumptions about die_id parsing in
> the sysfs that aren't true for Power hosts. In both Power8
> and Power9, running 5.6 and 4.18 kernel respectively,
> 'die_id' is set to -1:
>
> $ cat /sys/devices/system/cpu/cpu0/topology/die_id
> -1
>
> This breaks virHostCPUGetDie() parsing because it is trying to
> retrieve an unsigned integer, causing problems during VM start:
>
> virFileReadValueUint:4128 : internal error: Invalid unsigned integer
> value '-1' in file '/sys/devices/system/cpu/cpu0/topology/die_id'
>
> This isn't necessarily a PowerPC only behavior. Linux kernel commit
> 0e344d8c70 added in the former Documentation/cputopology.txt, now
> Documentation/admin-guide/cputopology.rst, that:
>
> ---
This line makes git-am cut the commit message here. I'm dropping it and
increasing indend of the paragraph below.
> To be consistent on all architectures, include/linux/topology.h
> provides default definitions for any of the above macros that are
> not defined by include/asm-XXX/topology.h:
>
> 1) topology_physical_package_id: -1
> 2) topology_die_id: -1
> (...)
> ---
>
> This means that it can be expected that an architecture that
> does not implement the die_id element will mark it as -1 in
> sysfs.
>
> Let's change the parsing being done in virHostCPUGetDie(), reading
> die_id as an integer instead of unsigned int. In case die_id is -1,
> default it to zero like the case of file not found.
>
> Fixes: 7b79ee2f78bbf2af76df2f6466919e19ae05aeeb
> Signed-off-by: Daniel Henrique Barboza
> ---
> src/util/virhostcpu.c | 21 ++---
> 1 file changed, 14 insertions(+), 7 deletions(-)
Reviewed-by: Michal Privoznik
and pushed.
Michal
On Tue, Mar 17, 2020 at 09:42:30 +0100, Michal Privoznik wrote:
> On 17. 3. 2020 9:26, Peter Krempa wrote:
> > On Tue, Mar 17, 2020 at 09:20:07 +0100, Michal Privoznik wrote:
> >> On 17. 3. 2020 8:52, Peter Krempa wrote:
[...]
> >> I don't think I share this view. The way qemu developers notify us is
> >> cross-posting to libvir-list. They can still do that and with the
> >> traffic on the list going down it will be pretty easy to spot these
> >> cross posts. Or am I missing something?
> >
> > Yes. As mentioned above though you need to be subscribed to the list
> > though. Also as mentioned above, that means that any serious developer
> > will need to be subscribe to the list. So all the point of not having to
> > subscribe to the list applies only to drive-by contributors.
> >
>
> Sure, but I thought this is expected. Every project has a place to
> discuss ideas, make decisions. Some use pull requests to do that (please
> don't), some have a mailing list. I see libvirt in the latter group. And
> to some extent, we are already in that situation. I mean, if I were a
> drive by contributor, I would subscribe to the list, post my patches and
> ignore the rest of incoming e-mails from the list.
>
> Maybe I'm misunderstanding and what is suggested it to move even all the
> discussion to gitlab. If that is the case, I stand by you. We should not
> do that. But if we are moving just the repo then I guess it is fine.
See the second paragraph of another subthread of this discussion:
https://www.redhat.com/archives/libvir-list/2020-March/msg00196.html
I wanted to raise this issue separately so that it's not burried. At any
rate, the idea of switching to the forge is to stop using email.
While this might appeal to upper layer developers where the cool new
deveopment approaches are used, we should not jeopardize our
relationship with qemu as we are trying to offer them our value
especially in the region of deprecation and changes of interfaces which
we are making transparent to users.
And if we are not going to kill off the libvir-list, the wins described
in the original thread are not as clear cut, as it just adds things you
have to watch/deal with, but for any real work, you'll have to endure
the pain from both email and web-based forges.
On 17. 3. 2020 9:26, Peter Krempa wrote:
> On Tue, Mar 17, 2020 at 09:20:07 +0100, Michal Privoznik wrote:
>> On 17. 3. 2020 8:52, Peter Krempa wrote:
>>> On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
We've discussed the idea of replacing our mailing list review workflow with
a merge request workflow in various places, over the last 6 months or so,
but never made a concrete decision.
>>>
>>> One other thing that worries me about this is that we've finally
>>> established a way close to qemu developers for notifying us if they are
>>> going to deprecate something or change something important.
>>>
>>> With moving development to some random web page with non-standard
>>> interfaces this will just mean that the notifications in this process
>>> will either stay on the old mailing list or be forgotten if we don't act
>>> on them.
>>>
>>> Moving development to some other place will in this regard just mean
>>> that we'll have to watch two places at the same time.
>>>
>>> While this seems to be a very low impact thing, the advantages of the
>>> new process you've outlined will only ever apply to drive-by
>>> contributors. Anybody wanting to take it seriously will necessarily need
>>> to subscribe to the mailing list anyways.
>>>
>>> In the end I just don't want to destroy the relationship with qemu
>>> developers by not acting on the notifications of change they send to us.
>>>
>>>
>>
>> I don't think I share this view. The way qemu developers notify us is
>> cross-posting to libvir-list. They can still do that and with the
>> traffic on the list going down it will be pretty easy to spot these
>> cross posts. Or am I missing something?
>
> Yes. As mentioned above though you need to be subscribed to the list
> though. Also as mentioned above, that means that any serious developer
> will need to be subscribe to the list. So all the point of not having to
> subscribe to the list applies only to drive-by contributors.
>
Sure, but I thought this is expected. Every project has a place to
discuss ideas, make decisions. Some use pull requests to do that (please
don't), some have a mailing list. I see libvirt in the latter group. And
to some extent, we are already in that situation. I mean, if I were a
drive by contributor, I would subscribe to the list, post my patches and
ignore the rest of incoming e-mails from the list.
Maybe I'm misunderstanding and what is suggested it to move even all the
discussion to gitlab. If that is the case, I stand by you. We should not
do that. But if we are moving just the repo then I guess it is fine.
Michal
On Tue, Mar 17, 2020 at 09:20:07 +0100, Michal Privoznik wrote:
> On 17. 3. 2020 8:52, Peter Krempa wrote:
> > On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
> >> We've discussed the idea of replacing our mailing list review workflow with
> >> a merge request workflow in various places, over the last 6 months or so,
> >> but never made a concrete decision.
> >
> > One other thing that worries me about this is that we've finally
> > established a way close to qemu developers for notifying us if they are
> > going to deprecate something or change something important.
> >
> > With moving development to some random web page with non-standard
> > interfaces this will just mean that the notifications in this process
> > will either stay on the old mailing list or be forgotten if we don't act
> > on them.
> >
> > Moving development to some other place will in this regard just mean
> > that we'll have to watch two places at the same time.
> >
> > While this seems to be a very low impact thing, the advantages of the
> > new process you've outlined will only ever apply to drive-by
> > contributors. Anybody wanting to take it seriously will necessarily need
> > to subscribe to the mailing list anyways.
> >
> > In the end I just don't want to destroy the relationship with qemu
> > developers by not acting on the notifications of change they send to us.
> >
> >
>
> I don't think I share this view. The way qemu developers notify us is
> cross-posting to libvir-list. They can still do that and with the
> traffic on the list going down it will be pretty easy to spot these
> cross posts. Or am I missing something?
Yes. As mentioned above though you need to be subscribed to the list
though. Also as mentioned above, that means that any serious developer
will need to be subscribe to the list. So all the point of not having to
subscribe to the list applies only to drive-by contributors.
On 17. 3. 2020 8:52, Peter Krempa wrote:
> On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
>> We've discussed the idea of replacing our mailing list review workflow with
>> a merge request workflow in various places, over the last 6 months or so,
>> but never made a concrete decision.
>
> One other thing that worries me about this is that we've finally
> established a way close to qemu developers for notifying us if they are
> going to deprecate something or change something important.
>
> With moving development to some random web page with non-standard
> interfaces this will just mean that the notifications in this process
> will either stay on the old mailing list or be forgotten if we don't act
> on them.
>
> Moving development to some other place will in this regard just mean
> that we'll have to watch two places at the same time.
>
> While this seems to be a very low impact thing, the advantages of the
> new process you've outlined will only ever apply to drive-by
> contributors. Anybody wanting to take it seriously will necessarily need
> to subscribe to the mailing list anyways.
>
> In the end I just don't want to destroy the relationship with qemu
> developers by not acting on the notifications of change they send to us.
>
>
I don't think I share this view. The way qemu developers notify us is
cross-posting to libvir-list. They can still do that and with the
traffic on the list going down it will be pretty easy to spot these
cross posts. Or am I missing something?
Michal
On Fri, Mar 06, 2020 at 11:44:07 +, Daniel Berrange wrote:
> We've discussed the idea of replacing our mailing list review workflow with
> a merge request workflow in various places, over the last 6 months or so,
> but never made a concrete decision.
One other thing that worries me about this is that we've finally
established a way close to qemu developers for notifying us if they are
going to deprecate something or change something important.
With moving development to some random web page with non-standard
interfaces this will just mean that the notifications in this process
will either stay on the old mailing list or be forgotten if we don't act
on them.
Moving development to some other place will in this regard just mean
that we'll have to watch two places at the same time.
While this seems to be a very low impact thing, the advantages of the
new process you've outlined will only ever apply to drive-by
contributors. Anybody wanting to take it seriously will necessarily need
to subscribe to the mailing list anyways.
In the end I just don't want to destroy the relationship with qemu
developers by not acting on the notifications of change they send to us.
