[libvirt] [PATCH 0/3] qemu: Drop some cruft

Andrea Bolognani posted 3 patches 5 years, 7 months ago
Failed in applying to current master (apply log)
src/qemu/qemu_capabilities.c                  | 27 +++++--------------
src/qemu/qemu_capabilities.h                  |  4 +--
src/qemu/qemu_command.c                       | 14 +---------
.../caps_1.5.3.x86_64.xml                     |  2 --
.../caps_1.6.0.x86_64.xml                     |  2 --
.../caps_1.7.0.x86_64.xml                     |  2 --
.../caps_2.1.1.x86_64.xml                     |  2 --
.../caps_2.10.0.aarch64.xml                   |  2 --
.../caps_2.10.0.ppc64.xml                     |  2 --
.../caps_2.10.0.s390x.xml                     |  2 --
.../caps_2.10.0.x86_64.xml                    |  2 --
.../caps_2.11.0.s390x.xml                     |  2 --
.../caps_2.11.0.x86_64.xml                    |  2 --
.../caps_2.12.0.aarch64.xml                   |  2 --
.../caps_2.12.0.ppc64.xml                     |  2 --
.../caps_2.12.0.s390x.xml                     |  2 --
.../caps_2.12.0.x86_64.xml                    |  2 --
.../caps_2.4.0.x86_64.xml                     |  2 --
.../caps_2.5.0.x86_64.xml                     |  2 --
.../caps_2.6.0.aarch64.xml                    |  2 --
.../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  2 --
.../caps_2.6.0.x86_64.xml                     |  2 --
.../qemucapabilitiesdata/caps_2.7.0.s390x.xml |  2 --
.../caps_2.7.0.x86_64.xml                     |  2 --
.../qemucapabilitiesdata/caps_2.8.0.s390x.xml |  2 --
.../caps_2.8.0.x86_64.xml                     |  2 --
.../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  2 --
.../qemucapabilitiesdata/caps_2.9.0.s390x.xml |  2 --
.../caps_2.9.0.x86_64.xml                     |  2 --
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  2 --
.../caps_3.0.0.riscv32.xml                    |  2 --
.../caps_3.0.0.riscv64.xml                    |  2 --
.../caps_3.0.0.x86_64.xml                     |  2 --
tests/qemuxml2argvtest.c                      |  6 ++---
34 files changed, 12 insertions(+), 99 deletions(-)
[libvirt] [PATCH 0/3] qemu: Drop some cruft
Posted by Andrea Bolognani 5 years, 7 months ago
No longer needed since we bumped our minimum QEMU version
to 1.5.0.

Andrea Bolognani (3):
  qemu: Drop QEMU_CAPS_VNC_WEBSOCKET
  qemu: Drop QEMU_CAPS_CHARDEV_SPICEPORT
  qemu: Drop redundant version checks

 src/qemu/qemu_capabilities.c                  | 27 +++++--------------
 src/qemu/qemu_capabilities.h                  |  4 +--
 src/qemu/qemu_command.c                       | 14 +---------
 .../caps_1.5.3.x86_64.xml                     |  2 --
 .../caps_1.6.0.x86_64.xml                     |  2 --
 .../caps_1.7.0.x86_64.xml                     |  2 --
 .../caps_2.1.1.x86_64.xml                     |  2 --
 .../caps_2.10.0.aarch64.xml                   |  2 --
 .../caps_2.10.0.ppc64.xml                     |  2 --
 .../caps_2.10.0.s390x.xml                     |  2 --
 .../caps_2.10.0.x86_64.xml                    |  2 --
 .../caps_2.11.0.s390x.xml                     |  2 --
 .../caps_2.11.0.x86_64.xml                    |  2 --
 .../caps_2.12.0.aarch64.xml                   |  2 --
 .../caps_2.12.0.ppc64.xml                     |  2 --
 .../caps_2.12.0.s390x.xml                     |  2 --
 .../caps_2.12.0.x86_64.xml                    |  2 --
 .../caps_2.4.0.x86_64.xml                     |  2 --
 .../caps_2.5.0.x86_64.xml                     |  2 --
 .../caps_2.6.0.aarch64.xml                    |  2 --
 .../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  2 --
 .../caps_2.6.0.x86_64.xml                     |  2 --
 .../qemucapabilitiesdata/caps_2.7.0.s390x.xml |  2 --
 .../caps_2.7.0.x86_64.xml                     |  2 --
 .../qemucapabilitiesdata/caps_2.8.0.s390x.xml |  2 --
 .../caps_2.8.0.x86_64.xml                     |  2 --
 .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  2 --
 .../qemucapabilitiesdata/caps_2.9.0.s390x.xml |  2 --
 .../caps_2.9.0.x86_64.xml                     |  2 --
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  2 --
 .../caps_3.0.0.riscv32.xml                    |  2 --
 .../caps_3.0.0.riscv64.xml                    |  2 --
 .../caps_3.0.0.x86_64.xml                     |  2 --
 tests/qemuxml2argvtest.c                      |  6 ++---
 34 files changed, 12 insertions(+), 99 deletions(-)

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Drop some cruft
Posted by Ján Tomko 5 years, 7 months ago
On Wed, Sep 12, 2018 at 02:48:51PM +0200, Andrea Bolognani wrote:
>No longer needed since we bumped our minimum QEMU version
>to 1.5.0.
>
>Andrea Bolognani (3):
>  qemu: Drop QEMU_CAPS_VNC_WEBSOCKET
>  qemu: Drop QEMU_CAPS_CHARDEV_SPICEPORT
>  qemu: Drop redundant version checks
>

Series:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Drop some cruft
Posted by John Ferlan 5 years, 7 months ago

On 09/12/2018 08:48 AM, Andrea Bolognani wrote:
> No longer needed since we bumped our minimum QEMU version
> to 1.5.0.
> 
> Andrea Bolognani (3):
>   qemu: Drop QEMU_CAPS_VNC_WEBSOCKET
>   qemu: Drop QEMU_CAPS_CHARDEV_SPICEPORT
>   qemu: Drop redundant version checks
> 
>  src/qemu/qemu_capabilities.c                  | 27 +++++--------------
>  src/qemu/qemu_capabilities.h                  |  4 +--
>  src/qemu/qemu_command.c                       | 14 +---------
>  .../caps_1.5.3.x86_64.xml                     |  2 --
>  .../caps_1.6.0.x86_64.xml                     |  2 --
>  .../caps_1.7.0.x86_64.xml                     |  2 --
>  .../caps_2.1.1.x86_64.xml                     |  2 --
>  .../caps_2.10.0.aarch64.xml                   |  2 --
>  .../caps_2.10.0.ppc64.xml                     |  2 --
>  .../caps_2.10.0.s390x.xml                     |  2 --
>  .../caps_2.10.0.x86_64.xml                    |  2 --
>  .../caps_2.11.0.s390x.xml                     |  2 --
>  .../caps_2.11.0.x86_64.xml                    |  2 --
>  .../caps_2.12.0.aarch64.xml                   |  2 --
>  .../caps_2.12.0.ppc64.xml                     |  2 --
>  .../caps_2.12.0.s390x.xml                     |  2 --
>  .../caps_2.12.0.x86_64.xml                    |  2 --
>  .../caps_2.4.0.x86_64.xml                     |  2 --
>  .../caps_2.5.0.x86_64.xml                     |  2 --
>  .../caps_2.6.0.aarch64.xml                    |  2 --
>  .../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  2 --
>  .../caps_2.6.0.x86_64.xml                     |  2 --
>  .../qemucapabilitiesdata/caps_2.7.0.s390x.xml |  2 --
>  .../caps_2.7.0.x86_64.xml                     |  2 --
>  .../qemucapabilitiesdata/caps_2.8.0.s390x.xml |  2 --
>  .../caps_2.8.0.x86_64.xml                     |  2 --
>  .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  2 --
>  .../qemucapabilitiesdata/caps_2.9.0.s390x.xml |  2 --
>  .../caps_2.9.0.x86_64.xml                     |  2 --
>  .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  2 --
>  .../caps_3.0.0.riscv32.xml                    |  2 --
>  .../caps_3.0.0.riscv64.xml                    |  2 --
>  .../caps_3.0.0.x86_64.xml                     |  2 --
>  tests/qemuxml2argvtest.c                      |  6 ++---
>  34 files changed, 12 insertions(+), 99 deletions(-)
> 

Any chance this can wait?  Would be nice to have other series posted
upstream that have changes to .xml and .replies files to add new
functionality be processed first.

There's a series to use - memfd from Marc-Andre Laurent that gets impacted.

Yeah - I know - first to merge wins...

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Drop some cruft
Posted by Ján Tomko 5 years, 7 months ago
On Wed, Sep 12, 2018 at 09:09:26AM -0400, John Ferlan wrote:
>
>
>On 09/12/2018 08:48 AM, Andrea Bolognani wrote:
>> No longer needed since we bumped our minimum QEMU version
>> to 1.5.0.
>>
>> Andrea Bolognani (3):
>>   qemu: Drop QEMU_CAPS_VNC_WEBSOCKET
>>   qemu: Drop QEMU_CAPS_CHARDEV_SPICEPORT
>>   qemu: Drop redundant version checks
>>
>>  src/qemu/qemu_capabilities.c                  | 27 +++++--------------
>>  src/qemu/qemu_capabilities.h                  |  4 +--
>>  src/qemu/qemu_command.c                       | 14 +---------
>>  .../caps_1.5.3.x86_64.xml                     |  2 --
>>  .../caps_1.6.0.x86_64.xml                     |  2 --
>>  .../caps_1.7.0.x86_64.xml                     |  2 --
>>  .../caps_2.1.1.x86_64.xml                     |  2 --
>>  .../caps_2.10.0.aarch64.xml                   |  2 --
>>  .../caps_2.10.0.ppc64.xml                     |  2 --
>>  .../caps_2.10.0.s390x.xml                     |  2 --
>>  .../caps_2.10.0.x86_64.xml                    |  2 --
>>  .../caps_2.11.0.s390x.xml                     |  2 --
>>  .../caps_2.11.0.x86_64.xml                    |  2 --
>>  .../caps_2.12.0.aarch64.xml                   |  2 --
>>  .../caps_2.12.0.ppc64.xml                     |  2 --
>>  .../caps_2.12.0.s390x.xml                     |  2 --
>>  .../caps_2.12.0.x86_64.xml                    |  2 --
>>  .../caps_2.4.0.x86_64.xml                     |  2 --
>>  .../caps_2.5.0.x86_64.xml                     |  2 --
>>  .../caps_2.6.0.aarch64.xml                    |  2 --
>>  .../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |  2 --
>>  .../caps_2.6.0.x86_64.xml                     |  2 --
>>  .../qemucapabilitiesdata/caps_2.7.0.s390x.xml |  2 --
>>  .../caps_2.7.0.x86_64.xml                     |  2 --
>>  .../qemucapabilitiesdata/caps_2.8.0.s390x.xml |  2 --
>>  .../caps_2.8.0.x86_64.xml                     |  2 --
>>  .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |  2 --
>>  .../qemucapabilitiesdata/caps_2.9.0.s390x.xml |  2 --
>>  .../caps_2.9.0.x86_64.xml                     |  2 --
>>  .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  2 --
>>  .../caps_3.0.0.riscv32.xml                    |  2 --
>>  .../caps_3.0.0.riscv64.xml                    |  2 --
>>  .../caps_3.0.0.x86_64.xml                     |  2 --
>>  tests/qemuxml2argvtest.c                      |  6 ++---
>>  34 files changed, 12 insertions(+), 99 deletions(-)
>>
>
>Any chance this can wait?  Would be nice to have other series posted
>upstream that have changes to .xml and .replies files to add new
>functionality be processed first.
>

This series does not touch the .replies files.

The .xml conflicts only adding new capabilities are trivial to resolve:
http://repo.or.cz/tomko-tools.git/blob/HEAD:/rcc

Jano

>There's a series to use - memfd from Marc-Andre Laurent that gets impacted.
>
>Yeah - I know - first to merge wins...
>
>John
>
>--
>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 0/3] qemu: Drop some cruft
Posted by Andrea Bolognani 5 years, 7 months ago
On Wed, 2018-09-12 at 09:09 -0400, John Ferlan wrote:
> Any chance this can wait?  Would be nice to have other series posted
> upstream that have changes to .xml and .replies files to add new
> functionality be processed first.
> 
> There's a series to use - memfd from Marc-Andre Laurent that gets impacted.

I see you've already had to rebase locally to apply the patches at
all, due to other changes being pushed in the meantime, so I don't
see how pushing this series too would make it any worse.

Also IIUC you've asked Marc-André to make some changes that depend
on *another series* of yours, that still hasn't been pushed, which
means a respin will be necessary either way, won't it?

All in all I see no reason to dealy pushing 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 0/3] qemu: Drop some cruft
Posted by John Ferlan 5 years, 7 months ago

On 09/12/2018 09:35 AM, Andrea Bolognani wrote:
> On Wed, 2018-09-12 at 09:09 -0400, John Ferlan wrote:
>> Any chance this can wait?  Would be nice to have other series posted
>> upstream that have changes to .xml and .replies files to add new
>> functionality be processed first.
>>
>> There's a series to use - memfd from Marc-Andre Laurent that gets impacted.
> 
> I see you've already had to rebase locally to apply the patches at
> all, due to other changes being pushed in the meantime, so I don't
> see how pushing this series too would make it any worse.
> 
> Also IIUC you've asked Marc-André to make some changes that depend
> on *another series* of yours, that still hasn't been pushed, which
> means a respin will be necessary either way, won't it?
> 
> All in all I see no reason to dealy pushing this.
> 

OK - go ahead. It was just a "would be nice" type inquiry. It's not the
first time capability changes cause conflicts and it won't be the last
unless we come up with a better process for them.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Drop some cruft
Posted by Ján Tomko 5 years, 7 months ago
On Wed, Sep 12, 2018 at 09:45:32AM -0400, John Ferlan wrote:
>
>
>On 09/12/2018 09:35 AM, Andrea Bolognani wrote:
>> On Wed, 2018-09-12 at 09:09 -0400, John Ferlan wrote:
>>> Any chance this can wait?  Would be nice to have other series posted
>>> upstream that have changes to .xml and .replies files to add new
>>> functionality be processed first.
>>>
>>> There's a series to use - memfd from Marc-Andre Laurent that gets impacted.

* Lureau

>>
>> I see you've already had to rebase locally to apply the patches at
>> all, due to other changes being pushed in the meantime, so I don't
>> see how pushing this series too would make it any worse.
>>
>> Also IIUC you've asked Marc-André to make some changes that depend
>> on *another series* of yours, that still hasn't been pushed, which
>> means a respin will be necessary either way, won't it?
>>
>> All in all I see no reason to dealy pushing this.
>>
>
>OK - go ahead. It was just a "would be nice" type inquiry.

I don't see how that negates the need to post another version,
because the intermediate diffs got too hard to follow.

>It's not the
>first time capability changes cause conflicts and it won't be the last
>unless we come up with a better process for them.

What's wrong with the current process?
The capabilities conflicts are trivial to resolve and we have automated
tools for most of it (VIR_TEST_REGENRATE_OUTPUT, group-qemu-caps.pl,
qemucapsfixreplies)

Also, Andrea's series seems to be innocent in this - the removed
capabilities do not alter .replies and the .[ch] file changes are
contained to the middle of the capabilities array, so they should
be resolved trivially by git.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Drop some cruft
Posted by John Ferlan 5 years, 7 months ago

On 09/12/2018 06:24 PM, Ján Tomko wrote:
> On Wed, Sep 12, 2018 at 09:45:32AM -0400, John Ferlan wrote:
>>
>>
>> On 09/12/2018 09:35 AM, Andrea Bolognani wrote:
>>> On Wed, 2018-09-12 at 09:09 -0400, John Ferlan wrote:
>>>> Any chance this can wait?  Would be nice to have other series posted
>>>> upstream that have changes to .xml and .replies files to add new
>>>> functionality be processed first.
>>>>
>>>> There's a series to use - memfd from Marc-Andre Laurent that gets
>>>> impacted.
> 
> * Lureau
> 

right, my fingers not typing what I'm thinking, mea culpa.

>>>
>>> I see you've already had to rebase locally to apply the patches at
>>> all, due to other changes being pushed in the meantime, so I don't
>>> see how pushing this series too would make it any worse.
>>>
>>> Also IIUC you've asked Marc-André to make some changes that depend
>>> on *another series* of yours, that still hasn't been pushed, which
>>> means a respin will be necessary either way, won't it?
>>>
>>> All in all I see no reason to dealy pushing this.
>>>
>>
>> OK - go ahead. It was just a "would be nice" type inquiry.
> 
> I don't see how that negates the need to post another version,
> because the intermediate diffs got too hard to follow.
> 

I don't see them as too hard to follow. In fact they're pretty simple as
long as you have seen the code. I saw no need to request a repost unless
it was because of the .replies differences. The hardest part is the move
of the code from domain_conf to qemu_domain and since I requested and
made those changes, I can see 'reason' to assist with adjusting the
patches I was reviewing. Managing the capabilities conflict was less
trivial and I certainly don't remember all the names of the various
tools that were created that are supposed to help in the endeavor.

>> It's not the
>> first time capability changes cause conflicts and it won't be the last
>> unless we come up with a better process for them.
> 
> What's wrong with the current process?

1. It's not orderly. Review of upstream series is "ad-hoc" at best. Some
series get immediate review, ack, and push. Sometimes certain series
languish for no apparent reason. When those series contain capabilities
changes it can quickly creates conflict.

2. At certain times there seems to be a "run" or "rush" to make
capabilities changes based on priorities perhaps not related to upstream
that create conflicts for patches that are languishing.

3. I think if we agree something is "good to add" capability wise and
even though all the code isn't ready, the capability changes could go
forward while perhaps the resolution of all review comments is handled.

Yes, it's more of a problem when .replies changes.

> The capabilities conflicts are trivial to resolve and we have automated
> tools for most of it (VIR_TEST_REGENRATE_OUTPUT, group-qemu-caps.pl,
> qemucapsfixreplies)
> 

OK sometimes trivial to resolve...  If one can keep up to date with
changes or 'git am' doesn't have conflicts trying to apply changes.  One
has to go back in time with git checkout to a series before the update
that caused the conflict, git apply patches, then
--set-upstream-to=origin/master, and then git pull --rebase.  And then
you get to try to use any of the tools to help resolve conflicts - if
you know about them and if they work; otherwise, it's a hand-edit or
request to repost.

Unless you know of some other neat trick or two...

How would qemucapsfixreplies fix the environment where git am fails to
do the merge. I didn't recall the name when processing the .replies
files. I'm also not sure having taken a quick peak at the code whether
it would have helped since there was "conflict" as a result of 4 replies
being removed and 1 reply being added in a very similar spot in the file.

As noted in the review - whenever I've created .replies files from
scratch using the tools - something in my process doesn't create output
in the exact format some seem to expect, so I've given up creating them
and just wait for someone else to do it.

> Also, Andrea's series seems to be innocent in this - the removed
> capabilities do not alter .replies and the .[ch] file changes are
> contained to the middle of the capabilities array, so they should
> be resolved trivially by git.
> 

True... These were, but considering they were reviewed 16 minutes after
posting and the previous series was reviewed similarly as quick and
those caused merge conflict issues - I reacted to just the fact that
capabilities changes were taking place and asked to consider the
"orderly processing" of posted patches. I didn't dig into them because I
was in the middle of something else.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list