[libvirt] [PATCH] news: Update release notes
Signed-off-by: --help <mprivozn@redhat.com> --- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 86a0c8d18..53bf9a49c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,28 @@ using the <code>cachetune</code> element in <code>cputune</code>. </description> </change> + <change> + <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description> + </change> + <change> + <summary> + virtlogd, virtlockd: Add support for admin protocol + </summary> + <description> + These two daemons now support admin protocol through which some admin + info can be gathered or some configuration tweaked on the fly. + </description> + </change> </section> <section title="Improvements"> <change> @@ -82,8 +104,88 @@ libxl: add support for setting clock offset and adjustment </summary> </change> + <change> + <summary> + Make port allocator global + </summary> + <description> + Up until now each driver had their own port allocator module. This + meant that info on port usage was not shared. Starting with this + release, the port allocator module is made global and therefore + drivers allocate ports from global pool. + </description> + </change> + <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description> + </change> </section> <section title="Bug fixes"> + <change> + <summary> + qemu: Check for unsafe migration more thoroughly + </summary> + <description> + If a domain disk is stored on local filesystem (e.g. ext4) but is + not being migrated it is very likely that domain is not able to + run on destination. Regardless of share/cache mode. + </description> + </change> + <change> + <summary> + qemu: Fix updating device with boot order + </summary> + <description> + Starting with 3.7.0 release updating any device with boot order would + fail with 'boot order X is already used by another device' while in + fact it was the very same device. + </description> + </change> + <change> + <summary> + virlog: determine the hostname on startup CVE-2018-6764 + </summary> + <description> + At later point it might not be possible or even safe to use + getaddrinfo(). It can in turn result in a load of NSS module which + can even be loaded from unsage guest filesystem and thus escape the + confinment of its container. + </description> + </change> + <change> + <summary> + qemu: Rework vCPU statistics fetching + </summary> + <description> + Fetching vCPU statistics was very expensive because it lead to waking + up vCPU threads in QEMU and thus it degraded performance. The code + was reworked so that fetching statistics does not wake up halted + vCPUs. + </description> + </change> + <change> + <summary> + qemu: unlink memory backing file on domain shutdown + </summary> + <description> + Depending on the filesystem where domain memory is stored, some files + might have been left behind. This is not a problem on hugetlbfs, but + it is a problem on regular filesystems like ext4. + </description> + </change> + <change> + <summary> + qemu: Fix shutting down domains in parallel + </summary> + <description> + If multiple domains were being shut down in parallel, libvirtd might + have deadlocked. + </description> + </change> </section> </release> <release version="v4.0.0" date="2018-01-19"> -- 2.16.1
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
Hmm.
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 86a0c8d18..53bf9a49c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,28 @@ using the <code>cachetune</code> element in <code>cputune</code>. </description> </change> + <change> + <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description>
Isn't this an internal change not really used for consumption of clients?
+ </change> + <change> + <summary> + virtlogd, virtlockd: Add support for admin protocol + </summary> + <description> + These two daemons now support admin protocol through which some admin + info can be gathered or some configuration tweaked on the fly. + </description> + </change> </section> <section title="Improvements"> <change> @@ -82,8 +104,88 @@ libxl: add support for setting clock offset and adjustment </summary> </change> + <change> + <summary> + Make port allocator global + </summary> + <description> + Up until now each driver had their own port allocator module. This + meant that info on port usage was not shared. Starting with this + release, the port allocator module is made global and therefore + drivers allocate ports from global pool. + </description> + </change> + <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
+ </change> </section> <section title="Bug fixes"> + <change> + <summary> + qemu: Check for unsafe migration more thoroughly + </summary> + <description> + If a domain disk is stored on local filesystem (e.g. ext4) but is + not being migrated it is very likely that domain is not able to + run on destination. Regardless of share/cache mode. + </description> + </change> + <change> + <summary> + qemu: Fix updating device with boot order + </summary> + <description> + Starting with 3.7.0 release updating any device with boot order would + fail with 'boot order X is already used by another device' while in + fact it was the very same device. + </description> + </change> + <change> + <summary> + virlog: determine the hostname on startup CVE-2018-6764 + </summary> + <description> + At later point it might not be possible or even safe to use + getaddrinfo(). It can in turn result in a load of NSS module which + can even be loaded from unsage guest filesystem and thus escape the + confinment of its container. + </description> + </change> + <change> + <summary> + qemu: Rework vCPU statistics fetching + </summary> + <description> + Fetching vCPU statistics was very expensive because it lead to waking + up vCPU threads in QEMU and thus it degraded performance. The code + was reworked so that fetching statistics does not wake up halted + vCPUs. + </description> + </change> + <change> + <summary> + qemu: unlink memory backing file on domain shutdown + </summary> + <description> + Depending on the filesystem where domain memory is stored, some files + might have been left behind. This is not a problem on hugetlbfs, but + it is a problem on regular filesystems like ext4. + </description> + </change> + <change> + <summary> + qemu: Fix shutting down domains in parallel + </summary> + <description> + If multiple domains were being shut down in parallel, libvirtd might + have deadlocked. + </description> + </change> </section> </release> <release version="v4.0.0" date="2018-01-19"> -- 2.16.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
Hmm.
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 86a0c8d18..53bf9a49c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,28 @@ using the <code>cachetune</code> element in <code>cputune</code>. </description> </change> + <change> + <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description>
Isn't this an internal change not really used for consumption of clients?
Not really. Try it yourself: virsh -c network:///system net-list --all
+ </change> + <change> + <summary> + virtlogd, virtlockd: Add support for admin protocol + </summary> + <description> + These two daemons now support admin protocol through which some admin + info can be gathered or some configuration tweaked on the fly. + </description> + </change> </section> <section title="Improvements"> <change> @@ -82,8 +104,88 @@ libxl: add support for setting clock offset and adjustment </summary> </change> + <change> + <summary> + Make port allocator global + </summary> + <description> + Up until now each driver had their own port allocator module. This + meant that info on port usage was not shared. Starting with this + release, the port allocator module is made global and therefore + drivers allocate ports from global pool. + </description> + </change> + <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
It added a lot of cases into our switches which are now safer. The problem with enums in switch() statements is we have to be 100% sure value fits into the enum. For instance: int x = VIR_DOMAIN_DEVICE_LAST + 1; switch ((virDomainDeviceType) x) { ... } is obviously problematic. And no, it's gcc 8. Michal (aka --help).
On Thu, Mar 01, 2018 at 15:12:39 +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
Hmm.
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 86a0c8d18..53bf9a49c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,28 @@ using the <code>cachetune</code> element in <code>cputune</code>. </description> </change> + <change> + <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description>
Isn't this an internal change not really used for consumption of clients?
Not really. Try it yourself:
virsh -c network:///system net-list --all
Well, that obviously has to work. But it's not exactly useful for general usage: $ virsh -c network:///system list --all error: Failed to list active domains error: this function is not supported by the connection driver: virConnectNumOfDomains [...]
+ <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
It added a lot of cases into our switches which are now safer. The problem with enums in switch() statements is we have to be 100% sure value fits into the enum. For instance:
int x = VIR_DOMAIN_DEVICE_LAST + 1;
switch ((virDomainDeviceType) x) { ... }
is obviously problematic.
But those are bug fixes and not improvements. In general the code got uglier since most switches got a 'default' case and an error report statement.
And no, it's gcc 8.
Oh right, it's not released yet so I did not find it ...
On Thu, Mar 01, 2018 at 03:28:57PM +0100, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 15:12:39 +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
Hmm.
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 86a0c8d18..53bf9a49c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,28 @@ using the <code>cachetune</code> element in <code>cputune</code>. </description> </change> + <change> + <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description>
Isn't this an internal change not really used for consumption of clients?
Not really. Try it yourself:
virsh -c network:///system net-list --all
Well, that obviously has to work. But it's not exactly useful for general usage:
$ virsh -c network:///system list --all error: Failed to list active domains error: this function is not supported by the connection driver: virConnectNumOfDomains
[...]
Note that in general we don't promise *any* API is supported by *any* URI. The set of APIs provided by any given driver are documented in our page https://libvirt.org/hvsupport.html So the fact that the network:/// URI doesn't support the listing of domains is not a bug. It is just an expected gap in the support matrix for that driver. IOW, this is largely a documentation task - I still need to provide docs for the secondary drivers to describe this better. 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 Thu, Mar 01, 2018 at 14:34:58 +0000, Daniel Berrange wrote:
On Thu, Mar 01, 2018 at 03:28:57PM +0100, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 15:12:39 +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
[...]
Isn't this an internal change not really used for consumption of clients?
Not really. Try it yourself:
virsh -c network:///system net-list --all
Well, that obviously has to work. But it's not exactly useful for general usage:
$ virsh -c network:///system list --all error: Failed to list active domains error: this function is not supported by the connection driver: virConnectNumOfDomains
[...]
Note that in general we don't promise *any* API is supported by *any* URI. The set of APIs provided by any given driver are documented in our page
https://libvirt.org/hvsupport.html
So the fact that the network:/// URI doesn't support the listing of domains is not a bug. It is just an expected gap in the support matrix for that driver.
IOW, this is largely a documentation task - I still need to provide docs for the secondary drivers to describe this better.
I'm not saying it's a bug. I'm just pointing out that it's not really useful by itself to open connection just to anything else than the VM driver itself. Libvirt is a library used to manage VMs and that's the main reason anybody will install it. I doubt that anybody would install libvirt to manage storage or firewall, since there are way better options to do so than our network/storage driver.
On Thu, Mar 01, 2018 at 03:41:00PM +0100, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:34:58 +0000, Daniel Berrange wrote:
On Thu, Mar 01, 2018 at 03:28:57PM +0100, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 15:12:39 +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
[...]
Isn't this an internal change not really used for consumption of clients?
Not really. Try it yourself:
virsh -c network:///system net-list --all
Well, that obviously has to work. But it's not exactly useful for general usage:
$ virsh -c network:///system list --all error: Failed to list active domains error: this function is not supported by the connection driver: virConnectNumOfDomains
[...]
Note that in general we don't promise *any* API is supported by *any* URI. The set of APIs provided by any given driver are documented in our page
https://libvirt.org/hvsupport.html
So the fact that the network:/// URI doesn't support the listing of domains is not a bug. It is just an expected gap in the support matrix for that driver.
IOW, this is largely a documentation task - I still need to provide docs for the secondary drivers to describe this better.
I'm not saying it's a bug. I'm just pointing out that it's not really useful by itself to open connection just to anything else than the VM driver itself. Libvirt is a library used to manage VMs and that's the main reason anybody will install it. I doubt that anybody would install libvirt to manage storage or firewall, since there are way better options to do so than our network/storage driver.
It isn't so compelling right now, but once we split libvirtd into one daemon per driver, it will be more useful to use the specific URIs if wanting to use the secondary drivers. It will let us containerize the daemons separately. It will also allow the use of real networking for the qemu://session daemon without setuid helpers. eg virt-manager can connect to network:///system to setup networking, to go with its connection to qemu:///session. 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 Thu, Mar 01, 2018 at 14:51:45 +0000, Daniel Berrange wrote:
On Thu, Mar 01, 2018 at 03:41:00PM +0100, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:34:58 +0000, Daniel Berrange wrote:
On Thu, Mar 01, 2018 at 03:28:57PM +0100, Peter Krempa wrote:
[...]
I'm not saying it's a bug. I'm just pointing out that it's not really useful by itself to open connection just to anything else than the VM driver itself. Libvirt is a library used to manage VMs and that's the main reason anybody will install it. I doubt that anybody would install libvirt to manage storage or firewall, since there are way better options to do so than our network/storage driver.
It isn't so compelling right now, but once we split libvirtd into one daemon per driver, it will be more useful to use the specific URIs if wanting to use the secondary drivers. It will let us containerize the daemons separately. It will also allow the use of real networking for the qemu://session daemon without setuid helpers. eg virt-manager can connect to network:///system to setup networking, to go with its connection to qemu:///session.
Well that's actually going to be pretty nice, but at this point the connections are not really worth as much fame. Since this is the notes file, nobody will read it once they'll be actually made useful in other cases than the internal connections.
On 03/01/2018 04:59 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:51:45 +0000, Daniel Berrange wrote:
On Thu, Mar 01, 2018 at 03:41:00PM +0100, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:34:58 +0000, Daniel Berrange wrote:
On Thu, Mar 01, 2018 at 03:28:57PM +0100, Peter Krempa wrote:
[...]
I'm not saying it's a bug. I'm just pointing out that it's not really useful by itself to open connection just to anything else than the VM driver itself. Libvirt is a library used to manage VMs and that's the main reason anybody will install it. I doubt that anybody would install libvirt to manage storage or firewall, since there are way better options to do so than our network/storage driver.
It isn't so compelling right now, but once we split libvirtd into one daemon per driver, it will be more useful to use the specific URIs if wanting to use the secondary drivers. It will let us containerize the daemons separately. It will also allow the use of real networking for the qemu://session daemon without setuid helpers. eg virt-manager can connect to network:///system to setup networking, to go with its connection to qemu:///session.
Well that's actually going to be pretty nice, but at this point the connections are not really worth as much fame. Since this is the notes file, nobody will read it once they'll be actually made useful in other cases than the internal connections.
What do you mean by 'made useful'? Fixing bugs in say storage driver? I don't really see what that has to do with anything because users can use storage driver through hypervisor connection already. So if storage driver is unusable, it's unusable regardless of connection URI. Michal
On Thu, Mar 01, 2018 at 03:12:39PM +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
Hmm.
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 86a0c8d18..53bf9a49c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,28 @@ using the <code>cachetune</code> element in <code>cputune</code>. </description> </change> + <change> + <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description>
Isn't this an internal change not really used for consumption of clients?
Not really. Try it yourself:
virsh -c network:///system net-list --all
Yes, that's explicitly intended to be usable by mgmt apps too. It would be useful to open the storage drivers in particular without a hypervisor, as many mgmt apps do storage operations on nodes which are not virt hosts.
+ <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
It added a lot of cases into our switches which are now safer. The problem with enums in switch() statements is we have to be 100% sure value fits into the enum. For instance:
int x = VIR_DOMAIN_DEVICE_LAST + 1;
switch ((virDomainDeviceType) x) { ... }
is obviously problematic. And no, it's gcc 8.
Well yes & no. GCC complained about cases where we fell-through case: statements, due to us not including enough cases. This caused me to notice the more general problem with us not handling enum values which didn't correspond to named constants. So the general addition of case/default everywhere was not specifically required by gcc 8. It is just something I chose todo to make us more robust after realizing the implications of what gcc8 identified. The warning flags we use to validate this have existed in ancient gcc versions. 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 03/01/2018 03:29 PM, Daniel P. Berrangé wrote:
On Thu, Mar 01, 2018 at 03:12:39PM +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
Hmm.
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
+ <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
It added a lot of cases into our switches which are now safer. The problem with enums in switch() statements is we have to be 100% sure value fits into the enum. For instance:
int x = VIR_DOMAIN_DEVICE_LAST + 1;
switch ((virDomainDeviceType) x) { ... }
is obviously problematic. And no, it's gcc 8.
Well yes & no. GCC complained about cases where we fell-through case: statements, due to us not including enough cases. This caused me to notice the more general problem with us not handling enum values which didn't correspond to named constants.
So the general addition of case/default everywhere was not specifically required by gcc 8. It is just something I chose todo to make us more robust after realizing the implications of what gcc8 identified. The warning flags we use to validate this have existed in ancient gcc versions.
Looks like Peter has opinion on this too. So should I leave this item out or reword it? Michal
On Thu, Mar 01, 2018 at 03:40:01PM +0100, Michal Privoznik wrote:
On 03/01/2018 03:29 PM, Daniel P. Berrangé wrote:
On Thu, Mar 01, 2018 at 03:12:39PM +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
Hmm.
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
+ <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
It added a lot of cases into our switches which are now safer. The problem with enums in switch() statements is we have to be 100% sure value fits into the enum. For instance:
int x = VIR_DOMAIN_DEVICE_LAST + 1;
switch ((virDomainDeviceType) x) { ... }
is obviously problematic. And no, it's gcc 8.
Well yes & no. GCC complained about cases where we fell-through case: statements, due to us not including enough cases. This caused me to notice the more general problem with us not handling enum values which didn't correspond to named constants.
So the general addition of case/default everywhere was not specifically required by gcc 8. It is just something I chose todo to make us more robust after realizing the implications of what gcc8 identified. The warning flags we use to validate this have existed in ancient gcc versions.
Looks like Peter has opinion on this too. So should I leave this item out or reword it?
Do we usually document when we just fix compiler warnings ? I'd just personally leave out, or just say "Fixed some compiler warnings that appear with GCC 8" and leave it at that. 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 Thu, 2018-03-01 at 14:42 +0000, Daniel P. Berrangé wrote:
Looks like Peter has opinion on this too. So should I leave this item out or reword it?
Do we usually document when we just fix compiler warnings ? I'd just personally leave out, or just say "Fixed some compiler warnings that appear with GCC 8" and leave it at that.
We usually document when we make a previously unusable OS / compiler usable. Now, if GCC 8 only reported warnings then that didn't really affect the ability to build from release tarballs, since we don't enable -Werror there; on the other hand, I don't see why we wouldn't document the fact that the latest and greatest GCC can be used to build libvirt. It seems release notes worthy to me, especially if we only spend a few words on it. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Mar 01, 2018 at 15:40:01 +0100, Michal Privoznik wrote:
On 03/01/2018 03:29 PM, Daniel P. Berrangé wrote:
On Thu, Mar 01, 2018 at 03:12:39PM +0100, Michal Privoznik wrote:
On 03/01/2018 02:15 PM, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 14:08:29 +0100, Michal Privoznik wrote:
Signed-off-by: --help <mprivozn@redhat.com>
[...]
problem with enums in switch() statements is we have to be 100% sure value fits into the enum. For instance:
int x = VIR_DOMAIN_DEVICE_LAST + 1;
switch ((virDomainDeviceType) x) { ... }
is obviously problematic. And no, it's gcc 8.
Well yes & no. GCC complained about cases where we fell-through case: statements, due to us not including enough cases. This caused me to notice the more general problem with us not handling enum values which didn't correspond to named constants.
So the general addition of case/default everywhere was not specifically required by gcc 8. It is just something I chose todo to make us more robust after realizing the implications of what gcc8 identified. The warning flags we use to validate this have existed in ancient gcc versions.
Looks like Peter has opinion on this too. So should I leave this item out or reword it?
Leave it in.
On Thu, 2018-03-01 at 14:15 +0100, Peter Krempa wrote:
+ <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description>
Isn't this an internal change not really used for consumption of clients?
That's what I thought as well. If so, it's not release notes material. [...]
+ <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
No, it was actually GCC 8.0, and there were some actual bugs being found thanks to it. You could maybe move it to the Bug Fixes section, but I'm happy either way. Everything else looks good, so with authorship info and S-o-b fixed Reviewed-by: --help <abologna@redhat.com> and safe for freeze. Thanks for accepting to be volunteered for this :) -- Andrea Bolognani / Red Hat / Virtualization
On 03/01/2018 03:20 PM, Andrea Bolognani wrote:
On Thu, 2018-03-01 at 14:15 +0100, Peter Krempa wrote:
+ <summary> + Allow opening secondary drivers + </summary> + <description> + Up until now it was possible to connect to only hypervisor drivers + (e.g. qemu:///system, lxc:///, vbox:///system, and so on). The + internal drivers (like network driver, node device driver, etc.) were + hidden from users and users could use them only indirectly. Starting + with this release new connection URIs are accepted. For instance + network:///system, storage:///system and so on. + </description>
Isn't this an internal change not really used for consumption of clients?
That's what I thought as well. If so, it's not release notes material.
[...]
+ <change> + <summary> + src: Enable building with GCC 8.0 + </summary> + <description> + GCC 8.0 added more warnings which found some genuine problems with our code. + </description>
I'm not sure whether that improved anything. Also wasn't that gcc 7?
No, it was actually GCC 8.0, and there were some actual bugs being found thanks to it. You could maybe move it to the Bug Fixes section, but I'm happy either way.
Everything else looks good, so with authorship info and S-o-b fixed
Reviewed-by: --help <abologna@redhat.com>
and safe for freeze.
Thanks for accepting to be volunteered for this :)
Thanks, I've pushed this. Michal
On Thu, Mar 01, 2018 at 02:08:29PM +0100, --help wrote:
Signed-off-by: --help <mprivozn@redhat.com>
You're going to have todo better than that to beat the exploits of Little Bobby Tables [1].
--- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
Regards, Daniel [1] https://xkcd.com/327/ -- |: 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 03/01/2018 02:27 PM, Daniel P. Berrangé wrote:
On Thu, Mar 01, 2018 at 02:08:29PM +0100, --help wrote:
Signed-off-by: --help <mprivozn@redhat.com>
You're going to have todo better than that to beat the exploits of Little Bobby Tables [1].
Oh snap. I was playing with git config when trying some guy on the list set their name and apparently I've messed mine :-) Little Bobby Tables is my favourite one. Michal
On Thu, Mar 01, 2018 at 02:08:29PM +0100, --help wrote:
Signed-off-by: --help <mprivozn@redhat.com> --- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
I think it might be worth mentioning the following bug fix as well: diff --git a/docs/news.xml b/docs/news.xml index 53bf9a49c..04318ca16 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -186,6 +186,18 @@ have deadlocked. </description> </change> + <change> + <summary> + nodedev: Update PCI mdev capabilities dynamically + </summary> + <description> + PCI devices may have other nested capabilities, like SRIOV and mdev + which depend on the device being plugged into the native vendor + driver. However, in case such a device is directly assigned to a guest + using VFIO driver, the device will naturally loose these capabilities + and libvirt needs to reflect that. + </description> + </change> </section> </release> <release version="v4.0.0" date="2018-01-19"> Thanks, Erik
On Thu, Mar 01, 2018 at 15:26:48 +0100, Erik Skultety wrote:
On Thu, Mar 01, 2018 at 02:08:29PM +0100, --help wrote:
Signed-off-by: --help <mprivozn@redhat.com> --- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
I think it might be worth mentioning the following bug fix as well:
diff --git a/docs/news.xml b/docs/news.xml index 53bf9a49c..04318ca16 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -186,6 +186,18 @@ have deadlocked. </description> </change> + <change> + <summary> + nodedev: Update PCI mdev capabilities dynamically + </summary> + <description> + PCI devices may have other nested capabilities, like SRIOV and mdev + which depend on the device being plugged into the native vendor + driver. However, in case such a device is directly assigned to a guest + using VFIO driver, the device will naturally loose these capabilities + and libvirt needs to reflect that. + </description> + </change> </section> </release> <release version="v4.0.0" date="2018-01-19">
News don't really have to be updated in a single change so you can propose this as a patch yourself.
On Thu, Mar 01, 2018 at 05:02:55PM +0100, Peter Krempa wrote:
On Thu, Mar 01, 2018 at 15:26:48 +0100, Erik Skultety wrote:
On Thu, Mar 01, 2018 at 02:08:29PM +0100, --help wrote:
Signed-off-by: --help <mprivozn@redhat.com> --- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
I think it might be worth mentioning the following bug fix as well:
diff --git a/docs/news.xml b/docs/news.xml index 53bf9a49c..04318ca16 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -186,6 +186,18 @@ have deadlocked. </description> </change> + <change> + <summary> + nodedev: Update PCI mdev capabilities dynamically + </summary> + <description> + PCI devices may have other nested capabilities, like SRIOV and mdev + which depend on the device being plugged into the native vendor + driver. However, in case such a device is directly assigned to a guest + using VFIO driver, the device will naturally loose these capabilities + and libvirt needs to reflect that. + </description> + </change> </section> </release> <release version="v4.0.0" date="2018-01-19">
News don't really have to be updated in a single change so you can propose this as a patch yourself.
Or he can just squash it in if there's not an issue with the wording itself, since the update hasn't been pushed yet. I really don't know what the purpose of this bikeshed was. Erik
On Thu, Mar 01, 2018 at 03:26:48PM +0100, Erik Skultety wrote:
On Thu, Mar 01, 2018 at 02:08:29PM +0100, --help wrote:
Signed-off-by: --help <mprivozn@redhat.com> --- docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+)
I think it might be worth mentioning the following bug fix as well:
diff --git a/docs/news.xml b/docs/news.xml index 53bf9a49c..04318ca16 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -186,6 +186,18 @@ have deadlocked. </description> </change> + <change> + <summary> + nodedev: Update PCI mdev capabilities dynamically + </summary> + <description> + PCI devices may have other nested capabilities, like SRIOV and mdev + which depend on the device being plugged into the native vendor + driver. However, in case such a device is directly assigned to a guest + using VFIO driver, the device will naturally loose these capabilities
s/loose/lose/ Jan
+ and libvirt needs to reflect that. + </description> + </change> </section> </release> <release version="v4.0.0" date="2018-01-19">
Thanks, Erik
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (7)
-
--help -
Andrea Bolognani -
Daniel P. Berrangé -
Erik Skultety -
Ján Tomko -
Michal Privoznik
-
Peter Krempa