[libvirt] [PATCH] news: Update release notes

--help posted 1 patch 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/78ba62bb4f9a7359c82e2056595d8e4ce6830969.1519909709.git.mprivozn@redhat.com
Test syntax-check passed
docs/news.xml | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
[libvirt] [PATCH] news: Update release notes
Posted by --help 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Peter Krempa 6 years, 1 month ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Michal Privoznik 6 years, 1 month ago
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).

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Peter Krempa 6 years, 1 month ago
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 ...

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Daniel P. Berrangé 6 years, 1 month ago
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Peter Krempa 6 years, 1 month ago
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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Daniel P. Berrangé 6 years, 1 month ago
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Peter Krempa 6 years, 1 month ago
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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Michal Privoznik 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Daniel P. Berrangé 6 years, 1 month ago
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Michal Privoznik 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Daniel P. Berrangé 6 years, 1 month ago
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Andrea Bolognani 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Peter Krempa 6 years, 1 month ago
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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Andrea Bolognani 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Michal Privoznik 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Daniel P. Berrangé 6 years, 1 month ago
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Michal Privoznik 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Erik Skultety 6 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Peter Krempa 6 years, 1 month ago
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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Erik Skultety 6 years, 1 month ago
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



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Update release notes
Posted by Ján Tomko 6 years, 1 month ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list