From: Anthony Liguori <anthony@codemonkey.ws>
To: Juan Quintela <quintela@redhat.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>, qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH 01/14] Introduce qemu_write_full()
Date: Tue, 19 Jan 2010 19:04:04 -0600 [thread overview]
Message-ID: <4B565684.2080809@codemonkey.ws> (raw)
In-Reply-To: <m38wbtljzp.fsf@neno.neno>
On 01/19/2010 06:04 PM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws> wrote:
>
>> On 01/19/2010 06:17 AM, Kirill A. Shutemov wrote:
>>
>>> On Tue, Jan 19, 2010 at 2:11 PM, Juan Quintela<quintela@redhat.com> wrote:
>>>
>>>
>>>> "Kirill A. Shutemov"<kirill@shutemov.name> wrote:
>>>>
>>>>
>>>>> A variant of write(2) which handles partial write.
>>>>>
>>>>> Signed-off-by: Kirill A. Shutemov<kirill@shutemov.name>
>>>>>
>>>>>
>>>> Hi
>>>>
>>>> Have you updated this series? Is there any reason that you know when
>>>> they haven't been picked?
>>>>
>>>>
>>> I don't know any reason, but I'm going to review it once again.
>>>
>>> I also have plan to get rid of -fno-strict-aliasing where it's possible.
>>>
>>>
>> I haven't reviewed the series in detail, but generally speaking I
>> don't feel that good about these sort of series.
>>
>> You're essentially adding dummy error handling to quiet the compiler.
>> That's worse than just disabling -Werror because at least you aren't
>> losing the information in the code.
>>
>> If you're going to update error handling, it should be part of an
>> effort to make code paths resilient to error. IOW, actually audit the
>> full error path of the function and make it deal with errors
>> gracefully.
>>
> I reviewed his series, and I reviewed callers. Please take a look at my
> improved series. Appart for the comments added there, I don't know what
> to do here:
>
> @@ -501,8 +501,11 @@ static void aio_signal_handler(int signum)
> {
> if (posix_aio_state) {
> char byte = 0;
> + ssize_t ret;
>
> - write(posix_aio_state->wfd,&byte, sizeof(byte));
> + ret = write(posix_aio_state->wfd,&byte, sizeof(byte));
> + if (ret< 0&& errno != EAGAIN)
> + die("write()");
> }
>
> if write() fails in a pipe in the signal handler, I am at a lost about
> what to do here.
>
That's nothing we can do. I guess exiting is reasonable.
Regards,
Anthony Liguori
next prev parent reply other threads:[~2010-01-20 1:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1262223199-19062-1-git-send-email-kirill@shutemov.name>
2010-01-19 12:11 ` [Qemu-devel] Re: [PATCH 01/14] Introduce qemu_write_full() Juan Quintela
2010-01-19 12:17 ` Kirill A. Shutemov
2010-01-19 18:50 ` Blue Swirl
2010-01-19 19:43 ` Anthony Liguori
2010-01-20 0:04 ` Juan Quintela
2010-01-20 1:04 ` Anthony Liguori [this message]
2010-01-20 1:30 ` Jamie Lokier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B565684.2080809@codemonkey.ws \
--to=anthony@codemonkey.ws \
--cc=kirill@shutemov.name \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.