[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] 'commit' error for 'all': No medium found
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] 'commit' error for 'all': No medium found |
Date: |
Mon, 25 Feb 2013 15:38:54 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2013-02-25 15:30, Markus Armbruster wrote:
> Jan Kiszka <address@hidden> writes:
>
>> On 2013-02-25 14:05, Jeff Cody wrote:
>>> On Sun, Feb 24, 2013 at 07:29:31PM +0100, Jan Kiszka wrote:
>>>> Hi,
>>>>
>>>> I'm seeing this with git head and 1.4. Apparently, commit on a
>>>> non-populated medium now generates this error instead of ignoring it
>>>> like in the past. As we stop iterating over the block devices while
>>>> doing "all", this may leaving uncommitted data behind.
>>>>
>>>> Didn't test, but I suspect 58513bde83 has something to do with it.
>>>>
>>>> Jan
>>>>
>>>
>>> Hi Jan,
>>>
>>> That commit just affected the reporting on the error - for the "all"
>>> case, bdrv_commit_all() still returned error on the first failure.
>>> When that happened 'commit' may have indicated success rather than
>>> error, depending on the error.
>>
>> Sorry, I just picked on the first commit that jumped into my eyes.
>>
>>>
>>> That would have also left uncommitted data behind, but done so
>>> silently and reported success.
>>>
>>> However, commit e8877497 added error checking to the bdrv_commit()
>>> return value in bdrv_commit_all() - before that bdrv_commit_all()
>>> ignored all error returns of bdrv_commit().
>>>
>>> Do you think there specific error returns that we should ignore then, in
>>> bdrv_commit_all(), such as -ENOMEDIUM?
>>
>> I think commit on a device without a medium should be a nop (as the
>
> Yes.
>
>> commit backlog is empty - provided it's correctly dropped on eject).
>
> Monitor commands eject and change close the backend. This does not
> collapse ("commit") any COWs into their backing images. If the backend
> was opened with BDRV_O_SNAPSHOT, the uncommitted updates are lost.
> That's a feature.
I don't disagree.
>
>>> Also, could you expand on what you mean by non-populated
>>> medium (test case) - is the error being returned "No medium found"?
>>
>> This is a default PC setup: the floppy tends to be not injected, thus
>> "commit all" now reports this error. If there is a blockdev after the
>> floppy in our list, it will not be committed this way. Luckly, the HD is
>> queued before the FD.
>
> Making bdrv_commit_all() check errors was a good idea, we just need to
> fix the regression in the !bs->drv case.
>
> I feel a bit queasy about detecting -ENOMEDIUM, because it's not obvious
> to me that bdrv_commit() can only return that when !bs->drv.
Can't we make bdrv_commit do the filtering and report success in the
absence of a medium? Or where can we reliably detect this state?
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
- [Qemu-devel] 'commit' error for 'all': No medium found, Jan Kiszka, 2013/02/24
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Jeff Cody, 2013/02/25
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Jan Kiszka, 2013/02/25
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Markus Armbruster, 2013/02/25
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Jeff Cody, 2013/02/25
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Jan Kiszka, 2013/02/25
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Markus Armbruster, 2013/02/25
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Kevin Wolf, 2013/02/26
- Re: [Qemu-devel] 'commit' error for 'all': No medium found, Stefan Hajnoczi, 2013/02/26
- Re: [Qemu-devel] 'commit' error for 'all': No medium found,
Jan Kiszka <=