So far we are setting only fake secret and storage drivers.
Therefore if the code wants to call a public NWFilter API (like
qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are
doing) the virGetConnectNWFilter() function will try to actually
spawn session daemon because there's no connection object set to
handle NWFilter driver.
Even though I haven't experienced the same problem with the rest
of the drivers (interface, network and node dev), the reasoning
above can be applied to them as well.
At the same time, now that connection object is registered for
the drivers, the public APIs will throw
virReportUnsupportedError(). And since we don't provide any error
func the error is printed to stderr. Fix this by setting dummy
error func.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
tests/qemuxml2argvtest.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a929e4314e..28073e2c40 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -471,6 +471,10 @@ testCompareXMLToArgv(const void *data)
conn->secretDriver = &fakeSecretDriver;
conn->storageDriver = &fakeStorageDriver;
+ virSetConnectInterface(conn);
+ virSetConnectNetwork(conn);
+ virSetConnectNWFilter(conn);
+ virSetConnectNodeDev(conn);
virSetConnectSecret(conn);
virSetConnectStorage(conn);
@@ -652,6 +656,8 @@ mymain(void)
return EXIT_FAILURE;
}
+ virTestQuiesceLibvirtErrors(true);
+
if (qemuTestDriverInit(&driver) < 0)
return EXIT_FAILURE;
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jul 12, 2018 at 09:37:47 +0200, Michal Privoznik wrote: > So far we are setting only fake secret and storage drivers. > Therefore if the code wants to call a public NWFilter API (like > qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are > doing) the virGetConnectNWFilter() function will try to actually > spawn session daemon because there's no connection object set to > handle NWFilter driver. > > Even though I haven't experienced the same problem with the rest > of the drivers (interface, network and node dev), the reasoning > above can be applied to them as well. > > At the same time, now that connection object is registered for > the drivers, the public APIs will throw > virReportUnsupportedError(). And since we don't provide any error > func the error is printed to stderr. Fix this by setting dummy > error func. > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > tests/qemuxml2argvtest.c | 6 ++++++ > 1 file changed, 6 insertions(+) [...] > > @@ -652,6 +656,8 @@ mymain(void) > return EXIT_FAILURE; > } > > + virTestQuiesceLibvirtErrors(true); > + NACK, this suppresses legitimate errors in the testsuite. I've mangled one of the XML files and ran the qemuxml2argvtest with VIR_TEST_DEBUG=1 and got: 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... FAILED 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP Without this patch I'd get: 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... libvirt: Domain Config error : unsupported configuration: unknown disk cache mode 'unafe' FAILED 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP > if (qemuTestDriverInit(&driver) < 0) > return EXIT_FAILURE; > > -- > 2.16.4 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/12/2018 10:01 AM, Peter Krempa wrote:
> On Thu, Jul 12, 2018 at 09:37:47 +0200, Michal Privoznik wrote:
>> So far we are setting only fake secret and storage drivers.
>> Therefore if the code wants to call a public NWFilter API (like
>> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are
>> doing) the virGetConnectNWFilter() function will try to actually
>> spawn session daemon because there's no connection object set to
>> handle NWFilter driver.
>>
>> Even though I haven't experienced the same problem with the rest
>> of the drivers (interface, network and node dev), the reasoning
>> above can be applied to them as well.
>>
>> At the same time, now that connection object is registered for
>> the drivers, the public APIs will throw
>> virReportUnsupportedError(). And since we don't provide any error
>> func the error is printed to stderr. Fix this by setting dummy
>> error func.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> tests/qemuxml2argvtest.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>
> [...]
>
>>
>> @@ -652,6 +656,8 @@ mymain(void)
>> return EXIT_FAILURE;
>> }
>>
>> + virTestQuiesceLibvirtErrors(true);
>> +
>
> NACK, this suppresses legitimate errors in the testsuite.
>
> I've mangled one of the XML files and ran the qemuxml2argvtest with
> VIR_TEST_DEBUG=1 and got:
>
> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP
> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... FAILED
> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP
>
> Without this patch I'd get:
>
> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP
> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... libvirt: Domain Config error : unsupported configuration: unknown disk cache mode 'unafe'
> FAILED
> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP
>
Well, without it I get:
tests $ ./qemuxml2argvtest
TEST: qemuxml2argvtest
._._._._..._._._._._._._._._._._._._._._ 40
._._._._._._._._._._._._._._._._._._._._ 80
._._._._._._._._._._._._._._._._._._._._ 120
._._._._._._._._._._._._._._._._._._._._ 160
._._._._._._._._._._._._._._._._._._._._ 200
._._._._._._._._._._._._._._._._..._._._ 240
.._._._._._...._._._._._._._._._._._._._ 280
._._._._._._._._._._._._._._._._._._._._ 320
._._._._._._._._._._._._._._._._._._._._ 360
._._._._._._._._._._._._._._._._._._._._ 400
._._._._._._._._._._._._._._._._._._._._ 440
._._._._._._._._libvirt: Network Filter Driver error : internal
error: unexpected nwfilter URI path '/session', try nwfilter:///system
libvirt: Network Filter Driver error : internal error: unexpected
nwfilter URI path '/session', try nwfilter:///system
libvirt: Network Filter Driver error : internal error: unexpected
nwfilter URI path '/session', try nwfilter:///system
libvirt: Network Filter Driver error : internal error: unexpected
nwfilter URI path '/session', try nwfilter:///system
libvirt: Network Filter Driver error : internal error: unexpected
nwfilter URI path '/session', try nwfilter:///system
._._._._._._._._._._._._ 480
So do you have any other idea? I came up with two already and neither of
them got through review. Just to remind everybody, we are possibly
touching live user data here so we need a resolution rather sooner than
later.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jul 12, 2018 at 10:51:33 +0200, Michal Privoznik wrote: > On 07/12/2018 10:01 AM, Peter Krempa wrote: > > On Thu, Jul 12, 2018 at 09:37:47 +0200, Michal Privoznik wrote: > >> So far we are setting only fake secret and storage drivers. > >> Therefore if the code wants to call a public NWFilter API (like > >> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are > >> doing) the virGetConnectNWFilter() function will try to actually > >> spawn session daemon because there's no connection object set to > >> handle NWFilter driver. > >> > >> Even though I haven't experienced the same problem with the rest > >> of the drivers (interface, network and node dev), the reasoning > >> above can be applied to them as well. > >> > >> At the same time, now that connection object is registered for > >> the drivers, the public APIs will throw > >> virReportUnsupportedError(). And since we don't provide any error > >> func the error is printed to stderr. Fix this by setting dummy > >> error func. > >> > >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > >> --- > >> tests/qemuxml2argvtest.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > > > > [...] > > > >> > >> @@ -652,6 +656,8 @@ mymain(void) > >> return EXIT_FAILURE; > >> } > >> > >> + virTestQuiesceLibvirtErrors(true); > >> + > > > > NACK, this suppresses legitimate errors in the testsuite. > > > > I've mangled one of the XML files and ran the qemuxml2argvtest with > > VIR_TEST_DEBUG=1 and got: > > > > 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP > > 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... FAILED > > 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP > > > > Without this patch I'd get: > > > > 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP > > 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... libvirt: Domain Config error : unsupported configuration: unknown disk cache mode 'unafe' > > FAILED > > 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP > > > > Well, without it I get: > > tests $ ./qemuxml2argvtest > TEST: qemuxml2argvtest > ._._._._..._._._._._._._._._._._._._._._ 40 > ._._._._._._._._._._._._._._._._._._._._ 80 > ._._._._._._._._._._._._._._._._._._._._ 120 > ._._._._._._._._._._._._._._._._._._._._ 160 > ._._._._._._._._._._._._._._._._._._._._ 200 > ._._._._._._._._._._._._._._._._..._._._ 240 > .._._._._._...._._._._._._._._._._._._._ 280 > ._._._._._._._._._._._._._._._._._._._._ 320 > ._._._._._._._._._._._._._._._._._._._._ 360 > ._._._._._._._._._._._._._._._._._._._._ 400 > ._._._._._._._._._._._._._._._._._._._._ 440 > ._._._._._._._._libvirt: Network Filter Driver error : internal > error: unexpected nwfilter URI path '/session', try nwfilter:///system > libvirt: Network Filter Driver error : internal error: unexpected > nwfilter URI path '/session', try nwfilter:///system > libvirt: Network Filter Driver error : internal error: unexpected > nwfilter URI path '/session', try nwfilter:///system > libvirt: Network Filter Driver error : internal error: unexpected > nwfilter URI path '/session', try nwfilter:///system > libvirt: Network Filter Driver error : internal error: unexpected > nwfilter URI path '/session', try nwfilter:///system > ._._._._._._._._._._._._ 480 > > > So do you have any other idea? I came up with two already and neither of > them got through review. Just to remind everybody, we are possibly > touching live user data here so we need a resolution rather sooner than > later. I specifically NACKd the part that installs the callback for suppressing error messages. The messages here need to be suppressed by some other way, but we should not decrease the debugability of tests. The error handler installation does not seem to have to do with live user data touching. I did not try to see what the other part of this patch does. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/12/2018 10:56 AM, Peter Krempa wrote: > On Thu, Jul 12, 2018 at 10:51:33 +0200, Michal Privoznik wrote: >> On 07/12/2018 10:01 AM, Peter Krempa wrote: >>> On Thu, Jul 12, 2018 at 09:37:47 +0200, Michal Privoznik wrote: >>>> So far we are setting only fake secret and storage drivers. >>>> Therefore if the code wants to call a public NWFilter API (like >>>> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are >>>> doing) the virGetConnectNWFilter() function will try to actually >>>> spawn session daemon because there's no connection object set to >>>> handle NWFilter driver. >>>> >>>> Even though I haven't experienced the same problem with the rest >>>> of the drivers (interface, network and node dev), the reasoning >>>> above can be applied to them as well. >>>> >>>> At the same time, now that connection object is registered for >>>> the drivers, the public APIs will throw >>>> virReportUnsupportedError(). And since we don't provide any error >>>> func the error is printed to stderr. Fix this by setting dummy >>>> error func. >>>> >>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>>> --- >>>> tests/qemuxml2argvtest.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>> >>> [...] >>> >>>> >>>> @@ -652,6 +656,8 @@ mymain(void) >>>> return EXIT_FAILURE; >>>> } >>>> >>>> + virTestQuiesceLibvirtErrors(true); >>>> + >>> >>> NACK, this suppresses legitimate errors in the testsuite. >>> >>> I've mangled one of the XML files and ran the qemuxml2argvtest with >>> VIR_TEST_DEBUG=1 and got: >>> >>> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP >>> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... FAILED >>> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP >>> >>> Without this patch I'd get: >>> >>> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP >>> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... libvirt: Domain Config error : unsupported configuration: unknown disk cache mode 'unafe' >>> FAILED >>> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP >>> >> >> Well, without it I get: >> >> tests $ ./qemuxml2argvtest >> TEST: qemuxml2argvtest >> ._._._._..._._._._._._._._._._._._._._._ 40 >> ._._._._._._._._._._._._._._._._._._._._ 80 >> ._._._._._._._._._._._._._._._._._._._._ 120 >> ._._._._._._._._._._._._._._._._._._._._ 160 >> ._._._._._._._._._._._._._._._._._._._._ 200 >> ._._._._._._._._._._._._._._._._..._._._ 240 >> .._._._._._...._._._._._._._._._._._._._ 280 >> ._._._._._._._._._._._._._._._._._._._._ 320 >> ._._._._._._._._._._._._._._._._._._._._ 360 >> ._._._._._._._._._._._._._._._._._._._._ 400 >> ._._._._._._._._._._._._._._._._._._._._ 440 >> ._._._._._._._._libvirt: Network Filter Driver error : internal >> error: unexpected nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> libvirt: Network Filter Driver error : internal error: unexpected >> nwfilter URI path '/session', try nwfilter:///system >> ._._._._._._._._._._._._ 480 >> >> >> So do you have any other idea? I came up with two already and neither of >> them got through review. Just to remind everybody, we are possibly >> touching live user data here so we need a resolution rather sooner than >> later. > > I specifically NACKd the part that installs the callback for suppressing > error messages. The messages here need to be suppressed by some other > way, but we should not decrease the debugability of tests. > > The error handler installation does not seem to have to do with live > user data touching. > > I did not try to see what the other part of this patch does. > Okay, I will drop it. The output of the test would be ugly then, but I don't care anymore. Also, can I get review on the rest of the patches please? I noticed some people started replying only to a single patch in a series leaving the rest unreviewed. I don't think that's good. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jul 12, 2018 at 11:00:42 +0200, Michal Privoznik wrote: > On 07/12/2018 10:56 AM, Peter Krempa wrote: [...] > Also, can I get review on the rest of the patches please? I noticed some > people started replying only to a single patch in a series leaving the > rest unreviewed. I don't think that's good. I did not plan to review this series. I think it's warranted to point out a problem with a patch even if you would not review that series otherwise. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/12/2018 11:08 AM, Peter Krempa wrote: > On Thu, Jul 12, 2018 at 11:00:42 +0200, Michal Privoznik wrote: >> On 07/12/2018 10:56 AM, Peter Krempa wrote: > > [...] > >> Also, can I get review on the rest of the patches please? I noticed some >> people started replying only to a single patch in a series leaving the >> rest unreviewed. I don't think that's good. > > I did not plan to review this series. I think it's warranted to point > out a problem with a patch even if you would not review that series > otherwise. > Well, unless reviewers are reading every e-mail in every thread, they will see a discussion to this thread thinking there is an review going on and thus skip to next (unreviewed) thread. This is usually my experience. So while pointing out problems is a good thing to do, leaving the rest unreviewed isn't IMO. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/12/2018 05:00 AM, Michal Privoznik wrote:
> On 07/12/2018 10:56 AM, Peter Krempa wrote:
>> On Thu, Jul 12, 2018 at 10:51:33 +0200, Michal Privoznik wrote:
>>> On 07/12/2018 10:01 AM, Peter Krempa wrote:
>>>> On Thu, Jul 12, 2018 at 09:37:47 +0200, Michal Privoznik wrote:
>>>>> So far we are setting only fake secret and storage drivers.
>>>>> Therefore if the code wants to call a public NWFilter API (like
>>>>> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are
>>>>> doing) the virGetConnectNWFilter() function will try to actually
>>>>> spawn session daemon because there's no connection object set to
>>>>> handle NWFilter driver.
>>>>>
>>>>> Even though I haven't experienced the same problem with the rest
>>>>> of the drivers (interface, network and node dev), the reasoning
>>>>> above can be applied to them as well.
>>>>>
>>>>> At the same time, now that connection object is registered for
>>>>> the drivers, the public APIs will throw
>>>>> virReportUnsupportedError(). And since we don't provide any error
>>>>> func the error is printed to stderr. Fix this by setting dummy
>>>>> error func.
>>>>>
>>>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>>>> ---
>>>>> tests/qemuxml2argvtest.c | 6 ++++++
>>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> [...]
>>>>
>>>>>
>>>>> @@ -652,6 +656,8 @@ mymain(void)
>>>>> return EXIT_FAILURE;
>>>>> }
>>>>>
>>>>> + virTestQuiesceLibvirtErrors(true);
>>>>> +
>>>>
>>>> NACK, this suppresses legitimate errors in the testsuite.
>>>>
>>>> I've mangled one of the XML files and ran the qemuxml2argvtest with
>>>> VIR_TEST_DEBUG=1 and got:
>>>>
>>>> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP
>>>> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... FAILED
>>>> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP
>>>>
>>>> Without this patch I'd get:
>>>>
>>>> 249) QEMU XML-2-startup-XML disk-drive-cache-directsync ... SKIP
>>>> 250) QEMU XML-2-ARGV disk-drive-cache-unsafe ... libvirt: Domain Config error : unsupported configuration: unknown disk cache mode 'unafe'
>>>> FAILED
>>>> 251) QEMU XML-2-startup-XML disk-drive-cache-unsafe ... SKIP
>>>>
>>>
>>> Well, without it I get:
>>>
>>> tests $ ./qemuxml2argvtest
>>> TEST: qemuxml2argvtest
>>> ._._._._..._._._._._._._._._._._._._._._ 40
>>> ._._._._._._._._._._._._._._._._._._._._ 80
>>> ._._._._._._._._._._._._._._._._._._._._ 120
>>> ._._._._._._._._._._._._._._._._._._._._ 160
>>> ._._._._._._._._._._._._._._._._._._._._ 200
>>> ._._._._._._._._._._._._._._._._..._._._ 240
>>> .._._._._._...._._._._._._._._._._._._._ 280
>>> ._._._._._._._._._._._._._._._._._._._._ 320
>>> ._._._._._._._._._._._._._._._._._._._._ 360
>>> ._._._._._._._._._._._._._._._._._._._._ 400
>>> ._._._._._._._._._._._._._._._._._._._._ 440
>>> ._._._._._._._._libvirt: Network Filter Driver error : internal
>>> error: unexpected nwfilter URI path '/session', try nwfilter:///system
>>> libvirt: Network Filter Driver error : internal error: unexpected
>>> nwfilter URI path '/session', try nwfilter:///system
>>> libvirt: Network Filter Driver error : internal error: unexpected
>>> nwfilter URI path '/session', try nwfilter:///system
>>> libvirt: Network Filter Driver error : internal error: unexpected
>>> nwfilter URI path '/session', try nwfilter:///system
>>> libvirt: Network Filter Driver error : internal error: unexpected
>>> nwfilter URI path '/session', try nwfilter:///system
>>> ._._._._._._._._._._._._ 480
>>>
FWIW: Using a "clean" build this same message appears for me before any
of these patches...
After this patch without Peter's NACK'd portion I get FOR TEST 469
(net-vhostuser-multiq):
libvirt: Network Filter Driver error : this function is not supported by
the connection driver: virNWFilterBindingLookupByPortDev
libvirt: Network Filter Driver error : this function is not supported by
the connection driver: virNWFilterBindingLookupByPortDev
libvirt: Network Filter Driver error : this function is not supported by
the connection driver: virNWFilterBindingLookupByPortDev
Running just test 469 (e.g. net-vhostuser-multiq w/
QEMU_CAPS_VHOSTUSER_MULTIQUEUE) using VIR_TEST_DEBUG=1 also shows:
2018-07-20 12:52:13.793+0000: 18540: warning :
qemuProcessStartWarnShmem:4923 : Detected vhost-user interface without
any shared memory, the interface might not be operational
2018-07-20 12:52:13.793+0000: 18540: error :
qemuBuildVhostuserCommandLine:8350 : unsupported configuration:
multi-queue is not supported for vhost-user with this QEMU binary
...
Since you're copying the virSetConnect*(conn) calls, I would think you
also need to create fake*Driver constructs as well; otherwise, for any
API called you'd get those function not supported messages. FWIW: I
believe they come from the virDomainConfNWFilterTeardownImpl code - it's
a real patience tester ;-).
FWIW: Investigating this is what sent me down the path of looking at
Jan's patches regarding needing jansson-devel because I was getting the
following results once Erik's patch to add a new capability (commit
7ab7d0ed49d8) were present:
./tests/qemuxml2argvtest
TEST: qemuxml2argvtest
._._._._ 8 FAIL
qemu was no longer being built, old images and libraries still existed,
and eventually failure was bound to occur.
>>>
>>> So do you have any other idea? I came up with two already and neither of
>>> them got through review. Just to remind everybody, we are possibly
>>> touching live user data here so we need a resolution rather sooner than
>>> later.
>>
>> I specifically NACKd the part that installs the callback for suppressing
>> error messages. The messages here need to be suppressed by some other
>> way, but we should not decrease the debugability of tests.
>>
>> The error handler installation does not seem to have to do with live
>> user data touching.
>>
>> I did not try to see what the other part of this patch does.
>>
>
> Okay, I will drop it. The output of the test would be ugly then, but I
> don't care anymore.
>
> Also, can I get review on the rest of the patches please? I noticed some
> people started replying only to a single patch in a series leaving the
> rest unreviewed. I don't think that's good.
>
This is always the ultimate conundrum - I agree with Michal that once
someone responds to a series that generally no one else looks. Well,
unless of course after a push it affects them negatively in some way
0-). On the other hand, since this patch seems separable from the rest
of the series (at least from my quick look), then perhaps multiple
series could have been posted. Of course that can get burdensome to
manage once reviews are given.
Still I think it'd be "good form" and "the polite thing" to do to at
least indicate that you looked at the rest of the patches in a series
and they're generally OK or indicate that you don't plan on reviewing
them. IMO, nothing worse than "fixing" the one patch that gets reviewed,
posting a new series, and then having the second patch get dinged when
the first review could have easily done the same thing.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.