diff mbox series

[PULL,v3,2/6] tests/9pfs: change qtest name prefix to synth

Message ID eefd2394efd6ba379d36d80c52de9ad956343b47.1603111175.git.qemu_oss@crudebyte.com (mailing list archive)
State New, archived
Headers show
Series [PULL,v3,1/6] 9pfs: suppress performance warnings on qtest runs | expand

Commit Message

Christian Schoenebeck Oct. 8, 2020, 6:34 p.m. UTC
All existing 9pfs test cases are using the 'synth' fs driver so far, which
means they are not accessing real files, but a purely simulated (in RAM
only) file system.

Let's make this clear by changing the prefix of the individual qtest case
names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
from upcoming new 9pfs test cases supposed to be using a different fs
driver.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 20, 2020, 7:36 a.m. UTC | #1
On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> All existing 9pfs test cases are using the 'synth' fs driver so far, which
> means they are not accessing real files, but a purely simulated (in RAM
> only) file system.
> 
> Let's make this clear by changing the prefix of the individual qtest case
> names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
> from upcoming new 9pfs test cases supposed to be using a different fs
> driver.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Message-Id: <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudebyte.com>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

Harmless, but don't need to sign twice ;)
Christian Schoenebeck Oct. 20, 2020, 9:43 a.m. UTC | #2
On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
> On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> > All existing 9pfs test cases are using the 'synth' fs driver so far, which
> > means they are not accessing real files, but a purely simulated (in RAM
> > only) file system.
> > 
> > Let's make this clear by changing the prefix of the individual qtest case
> > names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
> > from upcoming new 9pfs test cases supposed to be using a different fs
> > driver.
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > Message-Id:
> > <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudeby
> > te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> 
> Harmless, but don't need to sign twice ;)

Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as well, 
i.e. I thought this was intended to outline the patch author and submaintainer 
were the same person.

BTW I actually did not explicitly add the 2nd SOB. It was rather added by the 
patchwork client automatically. So maybe it should be fixed in the client to 
detect an already existing SOB line? Or am missing something here?

Best regards,
Christian Schoenebeck
Greg Kurz Oct. 20, 2020, 10 a.m. UTC | #3
On Tue, 20 Oct 2020 11:43:18 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
> > On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> > > All existing 9pfs test cases are using the 'synth' fs driver so far, which
> > > means they are not accessing real files, but a purely simulated (in RAM
> > > only) file system.
> > > 
> > > Let's make this clear by changing the prefix of the individual qtest case
> > > names from 'fs/' to 'synth/'. That way they'll be easily distinguishable
> > > from upcoming new 9pfs test cases supposed to be using a different fs
> > > driver.
> > > 
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > Message-Id:
> > > <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crudeby
> > > te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > 
> > Harmless, but don't need to sign twice ;)
> 
> Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as well, 
> i.e. I thought this was intended to outline the patch author and submaintainer 
> were the same person.
> 
> BTW I actually did not explicitly add the 2nd SOB. It was rather added by the 
> patchwork client automatically. So maybe it should be fixed in the client to 
> detect an already existing SOB line? Or am missing something here?
> 

Yeah this is the reason why my sob appears twice on patches authored by
me, and since this is harmless I never really investigated how to fix
pwclient :)

> Best regards,
> Christian Schoenebeck
> 
>
Christian Schoenebeck Oct. 20, 2020, 11:54 a.m. UTC | #4
On Dienstag, 20. Oktober 2020 12:00:57 CEST Greg Kurz wrote:
> On Tue, 20 Oct 2020 11:43:18 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
> > > On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> > > > All existing 9pfs test cases are using the 'synth' fs driver so far,
> > > > which
> > > > means they are not accessing real files, but a purely simulated (in
> > > > RAM
> > > > only) file system.
> > > > 
> > > > Let's make this clear by changing the prefix of the individual qtest
> > > > case
> > > > names from 'fs/' to 'synth/'. That way they'll be easily
> > > > distinguishable
> > > > from upcoming new 9pfs test cases supposed to be using a different fs
> > > > driver.
> > > > 
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > Message-Id:
> > > > <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crud
> > > > eby
> > > > te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > 
> > > Harmless, but don't need to sign twice ;)
> > 
> > Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as
> > well, i.e. I thought this was intended to outline the patch author and
> > submaintainer were the same person.
> > 
> > BTW I actually did not explicitly add the 2nd SOB. It was rather added by
> > the patchwork client automatically. So maybe it should be fixed in the
> > client to detect an already existing SOB line? Or am missing something
> > here?
> Yeah this is the reason why my sob appears twice on patches authored by
> me, and since this is harmless I never really investigated how to fix
> pwclient :)

Well, I would usually offer my 'I can look at it' at this point, but I am 
reluctant this time as I assume it will end up as my recently suggested libqos 
patches where I did not get any response from the officially assigned 
maintainers; not even a simple 'nack'.

Best regards,
Christian Schoenebeck
Philippe Mathieu-Daudé Oct. 21, 2020, 6:15 a.m. UTC | #5
Hi Cristian,

On 10/20/20 1:54 PM, Christian Schoenebeck wrote:
> On Dienstag, 20. Oktober 2020 12:00:57 CEST Greg Kurz wrote:
>> On Tue, 20 Oct 2020 11:43:18 +0200
>>
>> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>>> On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé wrote:
>>>> On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
>>>>> All existing 9pfs test cases are using the 'synth' fs driver so far,
>>>>> which
>>>>> means they are not accessing real files, but a purely simulated (in
>>>>> RAM
>>>>> only) file system.
>>>>>
>>>>> Let's make this clear by changing the prefix of the individual qtest
>>>>> case
>>>>> names from 'fs/' to 'synth/'. That way they'll be easily
>>>>> distinguishable
>>>>> from upcoming new 9pfs test cases supposed to be using a different fs
>>>>> driver.
>>>>>
>>>>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>>>> Message-Id:
>>>>> <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crud
>>>>> eby
>>>>> te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>>>
>>>> Harmless, but don't need to sign twice ;)
>>>
>>> Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as
>>> well, i.e. I thought this was intended to outline the patch author and
>>> submaintainer were the same person.
>>>
>>> BTW I actually did not explicitly add the 2nd SOB. It was rather added by
>>> the patchwork client automatically. So maybe it should be fixed in the
>>> client to detect an already existing SOB line? Or am missing something
>>> here?
>> Yeah this is the reason why my sob appears twice on patches authored by
>> me, and since this is harmless I never really investigated how to fix
>> pwclient :)
> 
> Well, I would usually offer my 'I can look at it' at this point, but I am
> reluctant this time as I assume it will end up as my recently suggested libqos
> patches where I did not get any response from the officially assigned
> maintainers; not even a simple 'nack'.

I was just watching your contributions and suggested an improvement
because something in your new workflow seems mis-configured (other
maintainers don't have this problem). I didn't asked you to fix a
bug in a different tool. I apologize if I was unclear and you
understood it this way.

Regarding your issue with a different series, I suppose you already
read:
https://wiki.qemu.org/Contribute/SubmitAPatch#If_your_patch_seems_to_have_been_ignored
and
https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor

You'll see that maintenance can be very time consuming, and we are
overcrowded from time to time when there is rush. I doubt maintainers
are ignoring your patches, as most of them try to do their best.
You might help them by reviewing patches for them, so they have time
to process your series.

Regards,

Phil.
Christian Schoenebeck Oct. 21, 2020, 10:45 a.m. UTC | #6
On Mittwoch, 21. Oktober 2020 08:15:55 CEST Philippe Mathieu-Daudé wrote:
> Hi Cristian,
> 
> On 10/20/20 1:54 PM, Christian Schoenebeck wrote:
> > On Dienstag, 20. Oktober 2020 12:00:57 CEST Greg Kurz wrote:
> >> On Tue, 20 Oct 2020 11:43:18 +0200
> >> 
> >> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> >>> On Dienstag, 20. Oktober 2020 09:36:10 CEST Philippe Mathieu-Daudé 
wrote:
> >>>> On 10/8/20 8:34 PM, Christian Schoenebeck wrote:
> >>>>> All existing 9pfs test cases are using the 'synth' fs driver so far,
> >>>>> which
> >>>>> means they are not accessing real files, but a purely simulated (in
> >>>>> RAM
> >>>>> only) file system.
> >>>>> 
> >>>>> Let's make this clear by changing the prefix of the individual qtest
> >>>>> case
> >>>>> names from 'fs/' to 'synth/'. That way they'll be easily
> >>>>> distinguishable
> >>>>> from upcoming new 9pfs test cases supposed to be using a different fs
> >>>>> driver.
> >>>>> 
> >>>>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> >>>>> Message-Id:
> >>>>> <e04e75acb849b085c6d6320b2433a15fa935bcff.1602182956.git.qemu_oss@crud
> >>>>> eby
> >>>>> te.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> >>>> 
> >>>> Harmless, but don't need to sign twice ;)
> >>> 
> >>> Ah, I thought that's the common way, as Greg's PRs contained 2 SOBs as
> >>> well, i.e. I thought this was intended to outline the patch author and
> >>> submaintainer were the same person.
> >>> 
> >>> BTW I actually did not explicitly add the 2nd SOB. It was rather added
> >>> by
> >>> the patchwork client automatically. So maybe it should be fixed in the
> >>> client to detect an already existing SOB line? Or am missing something
> >>> here?
> >> 
> >> Yeah this is the reason why my sob appears twice on patches authored by
> >> me, and since this is harmless I never really investigated how to fix
> >> pwclient :)
> > 
> > Well, I would usually offer my 'I can look at it' at this point, but I am
> > reluctant this time as I assume it will end up as my recently suggested
> > libqos patches where I did not get any response from the officially
> > assigned maintainers; not even a simple 'nack'.
> 
> I was just watching your contributions and suggested an improvement
> because something in your new workflow seems mis-configured (other
> maintainers don't have this problem). I didn't asked you to fix a
> bug in a different tool. I apologize if I was unclear and you
> understood it this way.

You actually did not raise that expectation to me Philippe, so definitely no 
need to apologize. But I appreciate it!

Correct me if I'm wrong, but AFAICS I'm actually not the only one being 
affected by this double-SOB issue. A short glimpse at the logs and I see for 
instance 3e7e134d827790c3714cae1d5b8aff8612000116 having it as well.

So I guess everyone having the following two options enabled in pwclientrc:

msgid=on
signoff=on

will have that issue.

> Regarding your issue with a different series, I suppose you already
> read:
> https://wiki.qemu.org/Contribute/SubmitAPatch#If_your_patch_seems_to_have_be
> en_ignored and
> https://wiki.qemu.org/Contribute/SubmitAPatch#Return_the_favor
> 
> You'll see that maintenance can be very time consuming, and we are
> overcrowded from time to time when there is rush. I doubt maintainers
> are ignoring your patches, as most of them try to do their best.
> You might help them by reviewing patches for them, so they have time
> to process your series.

Yes, I am aware of these. And once I got used to a new code base tree I also 
look at other ones' patches there.

I've recently been thinking whether it would be possible for QEMU 
submaintainers to make use of patchwork's status feature:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg737917.html

Maybe that could help preventing patches of high traffic components ending up 
unseen.

Best regards,
Christian Schoenebeck
diff mbox series

Patch

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index de30b717b6..3281153b9c 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -897,26 +897,26 @@  static void fs_readdir_split_512(void *obj, void *data,
 
 static void register_virtio_9p_test(void)
 {
-    qos_add_test("config", "virtio-9p", pci_config, NULL);
-    qos_add_test("fs/version/basic", "virtio-9p", fs_version, NULL);
-    qos_add_test("fs/attach/basic", "virtio-9p", fs_attach, NULL);
-    qos_add_test("fs/walk/basic", "virtio-9p", fs_walk, NULL);
-    qos_add_test("fs/walk/no_slash", "virtio-9p", fs_walk_no_slash,
+    qos_add_test("synth/config", "virtio-9p", pci_config, NULL);
+    qos_add_test("synth/version/basic", "virtio-9p", fs_version, NULL);
+    qos_add_test("synth/attach/basic", "virtio-9p", fs_attach, NULL);
+    qos_add_test("synth/walk/basic", "virtio-9p", fs_walk, NULL);
+    qos_add_test("synth/walk/no_slash", "virtio-9p", fs_walk_no_slash,
                  NULL);
-    qos_add_test("fs/walk/dotdot_from_root", "virtio-9p",
+    qos_add_test("synth/walk/dotdot_from_root", "virtio-9p",
                  fs_walk_dotdot, NULL);
-    qos_add_test("fs/lopen/basic", "virtio-9p", fs_lopen, NULL);
-    qos_add_test("fs/write/basic", "virtio-9p", fs_write, NULL);
-    qos_add_test("fs/flush/success", "virtio-9p", fs_flush_success,
+    qos_add_test("synth/lopen/basic", "virtio-9p", fs_lopen, NULL);
+    qos_add_test("synth/write/basic", "virtio-9p", fs_write, NULL);
+    qos_add_test("synth/flush/success", "virtio-9p", fs_flush_success,
                  NULL);
-    qos_add_test("fs/flush/ignored", "virtio-9p", fs_flush_ignored,
+    qos_add_test("synth/flush/ignored", "virtio-9p", fs_flush_ignored,
                  NULL);
-    qos_add_test("fs/readdir/basic", "virtio-9p", fs_readdir, NULL);
-    qos_add_test("fs/readdir/split_512", "virtio-9p",
+    qos_add_test("synth/readdir/basic", "virtio-9p", fs_readdir, NULL);
+    qos_add_test("synth/readdir/split_512", "virtio-9p",
                  fs_readdir_split_512, NULL);
-    qos_add_test("fs/readdir/split_256", "virtio-9p",
+    qos_add_test("synth/readdir/split_256", "virtio-9p",
                  fs_readdir_split_256, NULL);
-    qos_add_test("fs/readdir/split_128", "virtio-9p",
+    qos_add_test("synth/readdir/split_128", "virtio-9p",
                  fs_readdir_split_128, NULL);
 }