[Qemu-devel] First Patch, Requesting Comments‏

[Qemu-devel] First Patch, Requesting Comments‏

[Varad Gautam]

Hi! I'm new here, and am working on my first bug. I have posted a patch
for Bug#603872 [1]. It's incomplete right now, but please have a look and
tell me if I'm headed in the right direction. (I don't know if I can send
incomplete patches to the mailing list for suggestions or if I run into
some problems.)
 
Usecase: `qemu-img convert` with -p now shows the write speed.
 
I have a few doubts relating to the patch.
 
1. I'm calculating the speed using the time taken to run the for(;;)
at qemu-img.c:1477. I figured that every time this loop runs, n1
sectors are converted, and so I calculate the write_speed
accordingly. Is this correct?
 
2. I have changed qemu-progress.c:qemu_progress_print() to take in a
speed parameter, thinking that it would be the best option. Should I
do it some other way instead (maybe write another function to print
just speed)?
 
Also, what does IO_BUF_SIZE in the same file relate to?
 
Thanks.
Varad
 
[1] https://bugs.launchpad.net/qemu/+bug/603872

Re: [Qemu-devel] First Patch, Requesting Comments‏

[Max Filippov]

Hi Varad,

On Mon, Nov 18, 2013 at 5:49 PM, Varad Gautam <varadgautam@gmail.com> wrote:
>
> Hi! I'm new here, and am working on my first bug. I have posted a patch
> for Bug#603872 [1]. It's incomplete right now, but please have a look and
> tell me if I'm headed in the right direction. (I don't know if I can send
> incomplete patches to the mailing list for suggestions or if I run into
> some problems.)

Yes, you can post patches that you'd like to receive comments for,
just mark them appropriately, e.g. RFC instead of PATCH. Also cc
the maintainer of the code you're touching to increase chances for
review, see files MAINTAINERS and scripts/get_maintainer.pl.

> Usecase: `qemu-img convert` with -p now shows the write speed.
>
> I have a few doubts relating to the patch.

To discuss the code you really should post the patch, I couldn't
find any related code by your link.

> 1. I'm calculating the speed using the time taken to run the for(;;)
> at qemu-img.c:1477. I figured that every time this loop runs, n1
> sectors are converted, and so I calculate the write_speed
> accordingly. Is this correct?
>
> 2. I have changed qemu-progress.c:qemu_progress_print() to take in a
> speed parameter, thinking that it would be the best option. Should I
> do it some other way instead (maybe write another function to print
> just speed)?
>
> Also, what does IO_BUF_SIZE in the same file relate to?

Looks like disk I/O processing unit for qemu-img.

> Thanks.
> Varad
>
> [1] https://bugs.launchpad.net/qemu/+bug/603872

-- 
Thanks.
-- Max

Re: [Qemu-devel] First Patch, Requesting Comments

[Varad Gautam]

> To discuss the code you really should post the patch, I couldn't
> find any related code by your link.

Hi! I had posted the patch onto the list. I have also put it at [1] now.

Thanks.
Varad

[1] https://bugs.launchpad.net/qemu/+bug/603872

Re: [Qemu-devel] First Patch, Requesting Comments

[Andreas Färber]

Am 18.11.2013 16:59, schrieb Varad Gautam:
>> To discuss the code you really should post the patch, I couldn't
>> find any related code by your link.
> 
> Hi! I had posted the patch onto the list.

Unfortunately several copies of your mail arrived on the list today, and
only one actually has the patch it talks about threaded to it. ;)

But welcome to the community, and you'll surely find out how to optimize
your workflow for the second patch. :)

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] First Patch, Requesting Comments

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

On 11/18/2013 09:45 AM, Andreas Färber wrote:
> Am 18.11.2013 16:59, schrieb Varad Gautam:
>>> To discuss the code you really should post the patch, I couldn't
>>> find any related code by your link.
>>
>> Hi! I had posted the patch onto the list.
> 
> Unfortunately several copies of your mail arrived on the list today, and
> only one actually has the patch it talks about threaded to it. ;)

Often-times, this happens when a new contributor fails to realize that
the list is moderated, but that we will moderate non-subscriber's
messages through.  What commonly happens is that a new contributor sends
a message, sees nothing on the archives, so they then subscribe, then
send again under the assumption that it would help; and sometimes even
send a third time when getting impatient that it hasn't shown up in list
archives.  Then, a few hours later, when the moderator finally releases
the gates on the first-time post (a delay which happens whether you
posted as a subscriber or non-subscriber), all of the multiple pending
messages hit the list, making the new poster sound redundant.

Varad, as a new contributor, you are in a position to possibly help us:
what documentation pages did you read to learn where to post your
patches, so that we can try and modify those pages to give more hints to
help the next guy avoid the embarrassment of double posting?

> 
> But welcome to the community, and you'll surely find out how to optimize
> your workflow for the second patch. :)

Indeed. And if you haven't already found it,
http://wiki.qemu.org/Contribute/SubmitAPatch is a great resource (and
again, knowing what pages you HAVE found may help us figure out if we
can tweak those pages to more prominently point to this page).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] First Patch, Requesting Comments

[Varad Gautam]

On Mon, Nov 18, 2013 at 10:31 PM, Eric Blake <eblake@redhat.com> wrote:
> Often-times, this happens when a new contributor fails to realize that
> the list is moderated, but that we will moderate non-subscriber's
> messages through.  What commonly happens is that a new contributor sends
> a message, sees nothing on the archives, so they then subscribe, then
> send again under the assumption that it would help; and sometimes even
> send a third time when getting impatient that it hasn't shown up in list
> archives.  Then, a few hours later, when the moderator finally releases
> the gates on the first-time post (a delay which happens whether you
> posted as a subscriber or non-subscriber), all of the multiple pending
> messages hit the list, making the new poster sound redundant.

Thanks. I was wondering what had happened to the mails when suddenly I saw
a lot of them in my inbox!

> Varad, as a new contributor, you are in a position to possibly help us:
> what documentation pages did you read to learn where to post your
> patches, so that we can try and modify those pages to give more hints to
> help the next guy avoid the embarrassment of double posting?
>
>>
>> But welcome to the community, and you'll surely find out how to optimize
>> your workflow for the second patch. :)
>
> Indeed. And if you haven't already found it,
> http://wiki.qemu.org/Contribute/SubmitAPatch is a great resource (and
> again, knowing what pages you HAVE found may help us figure out if we
> can tweak those pages to more prominently point to this page).

Sure. I went through http://wiki.qemu.org/Documentation/GettingStartedDevelopers
and http://wiki.qemu.org/Contribute/SubmitAPatch wikis before sending out the
patch. I guess it'd be useful to change these to avoid such happenings.

Thanks for the reply.
Varad

Re: [Qemu-devel] First Patch, Requesting Comments

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1268 bytes --]

On 11/18/2013 11:41 AM, Varad Gautam wrote:
> On Mon, Nov 18, 2013 at 10:31 PM, Eric Blake <eblake@redhat.com> wrote:

>> Varad, as a new contributor, you are in a position to possibly help us:
>> what documentation pages did you read to learn where to post your
>> patches, so that we can try and modify those pages to give more hints to
>> help the next guy avoid the embarrassment of double posting?
>>

> Sure. I went through http://wiki.qemu.org/Documentation/GettingStartedDevelopers
> and http://wiki.qemu.org/Contribute/SubmitAPatch wikis before sending out the
> patch. I guess it'd be useful to change these to avoid such happenings.

It's a wiki, so anyone else can make further edits, but I added words to
both of these pages today.

One page I cannot edit (since I'm not a list moderator), but which would
also be good to modify, is the actual list information page:
https://lists.nongnu.org/mailman/listinfo/qemu-devel

[List moderators - you may be interested in how libvirt does it:
https://www.redhat.com/mailman/listinfo/libvir-list
the trick is to fill in the 'Info' box on the mailman admin General
Options page]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

[Qemu-devel] First Patch, Requesting Comments

[Varad Gautam]

[-- Attachment #1: Type: text/plain, Size: 993 bytes --]

Hi! I'm new here, and am working on my first bug. I have posted a patch
for Bug#603872 [1] to the list.. It's incomplete right now, but please
have a look and tell me if I'm headed in the right direction. (I don't
know if I can send incomplete patches to the mailing list for suggestions
or if I run into some problems.)

Usecase: `qemu-img convert` with -p now shows the write speed.

I have a few doubts relating to the patch.

1. I'm calculating the speed using the time taken to run the for(;;)
at qemu-img.c:1477. I figured that every time this loop runs, n1
sectors are converted, and so I calculate the write_speed
accordingly. Is this correct?

2. I have changed qemu-progress.c:qemu_progress_print() to take in a
speed parameter, thinking that it would be the best option. Should I
do it some other way instead (maybe write another function to print
just speed)?

Also, what does IO_BUF_SIZE in the same file relate to?

Thanks.
Varad

[1] https://bugs.launchpad.net/qemu/+bug/603872

[-- Attachment #2: Type: text/html, Size: 1215 bytes --]

[Qemu-devel] First Patch, Requesting Comments

[Varad Gautam]

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

Hi! I'm new here, and am working on my first bug. I have posted a patch
for Bug#603872 [1] to the list.. It's incomplete right now, but please
have a look and tell me if I'm headed in the right direction. (I don't

know if I can send incomplete patches to the mailing list for suggestions
or if I run into some problems.)

Usecase: `qemu-img convert` with -p now shows the write speed.

I have a few doubts relating to the patch.


1. I'm calculating the speed using the time taken to run the for(;;)
at qemu-img.c:1477. I figured that every time this loop runs, n1
sectors are converted, and so I calculate the write_speed
accordingly. Is this correct?


2. I have changed qemu-progress.c:qemu_progress_print() to take in a
speed parameter, thinking that it would be the best option. Should I
do it some other way instead (maybe write another function to print
just speed)?


Also, what does IO_BUF_SIZE in the same file relate to?

Thanks.
Varad

[1] https://bugs.launchpad.net/qemu/+bug/603872

[-- Attachment #2: Type: text/html, Size: 1223 bytes --]