On Fri, Nov 11, 2016 at 08:46:11PM +0100, Max Reitz wrote: > On 09.11.2016 20:15, Jeff Cody wrote: > > On Tue, Nov 08, 2016 at 08:14:58AM +0100, Markus Armbruster wrote: > >> Max Reitz <mre...@redhat.com> writes: > >> > >>> On 07.11.2016 09:20, Markus Armbruster wrote: > >>>> Max Reitz <mre...@redhat.com> writes: > >>>> > >>>>> On 03.11.2016 08:56, Markus Armbruster wrote: > >>>>>> Max Reitz <mre...@redhat.com> 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. > > > > I'm going to go ahead and apply the series; I think the relevant point > > for the commit message is that TFTP is not usable and never has been. If > > Max has no objections, I'll pull some wording in from his reply here into > > his commit message for patch 3, and squash all the patches into one. > > > > Max, any objections? > > No, that sounds good to me. Thank you very much! > > Max >
Thanks, Applied to my block branch (squashed, and commit message amended): git://github.com/codyprime/qemu-kvm-jtc.git block -Jeff