[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support" |
Date: |
Tue, 08 Nov 2016 08:14:58 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Max Reitz <address@hidden> writes:
> On 07.11.2016 09:20, Markus Armbruster wrote:
>> Max Reitz <address@hidden> writes:
>>
>>> On 03.11.2016 08:56, Markus Armbruster wrote:
>>>> Max Reitz <address@hidden> writes:
>>>>
>>>>> See patch 3 for the reason why we have actually never supported TFTP at
>>>>> all (except for very small files (i.e. below 256 kB or so)).
>>>>
>>>> Care to explain why it works "for very small files" in a bit more
>>>> detail? PATCH 3 gives a "does not support byte ranges" hint, but to go
>>>> from there to "very small files", you need to know more about how the
>>>> block layer works than I can remember right now.
>>>
>>> Our curl block drivers caches data and uses a readahead cache, which by
>>> default has a size of 256 kB. Therefore, if the start of the file is
>>> read first (which it usually is, if just for format probing), then the
>>> correct data will be read for that size.
>>>
>>> Yes, you can adjust the readahead size. No, I cannot guarantee that
>>> there are no users that just set readahead to the image size and thus
>>> made it work. I can't really imagine that, though, because at that point
>>> you can just copy the file to tmpfs and have the same result.
>>>
>>> Also, if I were a user, I probably wouldn't use 256 kB images, and thus
>>> I would just notice tftp to be broken. I don't think I would experiment
>>> with the readahead option to find out that it works if I set it to the
>>> image size and then just use it that way. I definitely think I would
>>> give up before that and just copy the file to the local system.
>>
>> I'm not trying to make you explain why it's okay to drop TFTP. I'm
>> trying to make you explain what exactly worked and what exactly didn't.
>> Such explanations generally involve a certain degree of "why".
>
> Well, I'm trying to explain both. :-)
>
>> Your first paragraph provides a few more hints, but I'm still guessing.
>> Here's my current best guess:
>>
>> * Commonly, images smaller than 256 KiB work, and larger images don't.
>
> Yes. Unless you set the "readahead" option to something different (it
> just defaults to 256 kB), then it'll commonly work for that images up to
> that size.
>
> Oh, and I just realized it's not called "readahead" for nothing: It gets
> added to the size of the read operation, so if your first read operation
> has a size of 1 GB... Well, then all of that will be correctly cached.
> So both the size and the offset of the first read operation are significant.
>
>> * "Don't work" means the block layer returns garbled data.
>
> Right. It will be data from the image, but not from the offset you want.
>
>> * "Commonly" means when the first read is for offset zero. Begs the
>> question when exactly that's the case. You mentioned format probing.
>> What if the user specified a format? It's okay not to answer this
>> question. I'm not demanding exhaustive analysis, I'm fishing for a
>> better commit message. Such a message may leave some of its questions
>> unanswered.
>
> Well, qcow2 will always start at offset zero anyway (because it reads
> the header first). For raw images, the offset can be anywhere, but if
> you're starting a VM from it, offset zero is obviously likely to be read
> first, too.
>
> (And as a side note, the first read operation for qcow2 images will
> always be 64 kB in size.)
>
> But, yes, for raw images the offset can be anywhere and if it is not
> zero, the answer what works and what doesn't becomes a bit more complicated:
>
> <optional>
> Suppose the first offset read from is 64k. curl will return data from
> offset 0 anyway, so it's pretty much garbage. But if you then do another
> read operation from 0, that will return correct data.
>
> If after that you try to read data from the area that has been covered
> by both read operations... Then it depends on which buffer the curl
> driver sees first, which is most likely the first one, i.e. you'll get
> broken data again.
> </optional>
There's a lovely addition to your commit message struggling to get out
of your reply.
- Re: [Qemu-block] [PATCH for-2.8? 1/3] qemu-options: Drop mentions of curl's TFTP support, (continued)
- [Qemu-block] [PATCH for-2.8? 2/3] qapi: Drop curl's TFTP protocol, Max Reitz, 2016/11/02
- [Qemu-block] [PATCH for-2.8? 3/3] block/curl: Drop TFTP "support", Max Reitz, 2016/11/02
- Re: [Qemu-block] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Jeff Cody, 2016/11/02
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Markus Armbruster, 2016/11/03
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Max Reitz, 2016/11/04
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Markus Armbruster, 2016/11/07
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Max Reitz, 2016/11/07
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support",
Markus Armbruster <=
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Jeff Cody, 2016/11/09
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Max Reitz, 2016/11/11
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Jeff Cody, 2016/11/14
Re: [Qemu-block] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Kevin Wolf, 2016/11/03
Re: [Qemu-block] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support", Stefan Hajnoczi, 2016/11/07