On 2013-06-24 Richard W.M. Jones wrote:
> I have now completed a simple random-access XZ NBD server plugin:
> 
> https://github.com/libguestfs/nbdkit/tree/master/plugins/xz
> 
> which may be of interest.  It works enough that I can read out some
> xz-compressed Windows guest disks, which is a fairly good test.

Nice that you got it working.

Below are a few more things that I noticed that you may or may not find
interesting.

"xz --block-size=SIZE" is available only in 5.1.x branch which is still
officially in alpha stage. I don't know if the required xz version is
worth mentioning in the docs. A modified version of 5.1.x is shipped in
Debian and maybe some other distros (Fedora maybe) ship 5.1.x too. In
practice it seems to work quite OK since I haven't got bug reports.

With "xz --block-size=SIZE" SIZE can be e.g. 16MiB which is easier to
type than 16777216. Most options in xz that take numbers accept such
suffixes. It is documented on the man page but it's not mentioned again
for each option (maybe it should be?).

I noticed that both list.c and your code lack a check for the
lzma_stream_flags.version field (see <lzma/stream_flags.h>). It doesn't
affect anything for now since the version is always 0. But it quite
probably won't always be zero in the future (e.g. if metadata support
is added to .xz) and then the current code may misbehave instead of
giving a clear error message. Here's what I added to list.c:

    
http://git.tukaani.org/?p=xz.git;a=commitdiff;h=84d2da6c9dc252f441deb7626c2522202b005d4d

xzfile.c lines 467-488:

  - Changing the action argument to LZMA_FINISH from LZMA_RUN when no
    more input is coming is fine but when decompressing blocks it is
    not required. It is fine to only use LZMA_RUN if you want.

  - After successful decoding, lzma_code() must have returned
    LZMA_STREAM_END. On line 488 the code accepts also LZMA_OK, which
    looks suspicious. In practice there is no bug because the
    "while" condition on line 486 ensures that the value cannot be
    LZMA_OK, making the check for LZMA_OK on the line 488 a no-op.

-- 
Lasse Collin  |  IRC: Larhzu @ IRCnet & Freenode

Reply via email to