Re: [Qemu-devel] [PATCH] qga/commands-posix: Fix bug in guest-fstrim

[Justin Ossevoort]

Hello,

I posted a PATCH v2 yesterday that does this. It returns the result per 
filesystem, with the name, mountpoint and filesystem type.

Eric requested output similar to fsinfo (that's why name and type is 
included), but I'm not really convinced it is meaningful. Especially 
'name' is something that has no real meaning, but determining it is one 
of the most likely failure cases for this operation. I included type 
because to complement the 3 properties of fsinfo, and since it at least 
conveys some information about a possible returned error for that 
mountpoint and it is trivially available.

But a FITRIM operation may be performed on any directory (not 
necessarily a mountpoint, though that is the sensible path to use when 
trimming all filesystems). And I can imagine someone extending the 
'guest-fstrim' operation to take an optional path(s) argument, but in 
that case you don't want to report name, mountpoint and type but only a 
"path" (which in the current implementation would be the mountpoint).

Another issue I ran into is that this is a batch operation. I changed 
the result to include a "result" enum (success / operation-failed) and 
if it is a "success" than the "trimmid" and "minimum" fields for that 
mountpoint are returned, if it is an "operation-failed" than the "error" 
(with the specific errno value) field is returned.
But I'm unsure if this is how one would want to use this return type. 
Pherhaps it would be better to drop the "result" field and mirror the 
basic return logic: "error" attribute on error and a "return" attribute 
with a dictionary containing the per mountpoint result?

Regards,

Justin

On 01-04-15 18:49, Paolo Bonzini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
>
>
> On 01/04/2015 14:33, Eric Blake wrote:
> > It's only a minor incompatibility, but a client that hard-codes
> > itself to parsing "returns":0 (that is, expecting a json-number)
> > will fail when talking to an older qemu that provided a json-object
> > instead; while a client that expects a json-object always and can
> > search for a "key":0 integer pair within that object that may or
> > may not be present (we already document that clients MUST be
> > prepared for dictionary members to be optional, but do NOT advise
> > that clients must be prepared for a change between fundamental JSON
> > types).  Yes, the backwards-incompatibility is a weak argument, but
> > as my pending series will be requiring all new commands to
> > whitelist a non-dict return, we might as well avoid making this
> > another case for that whitelist.
>
> That's true, and we might take the opportunity to return the number of
> trimmed bytes separately for each filesystem.
>
> Justin, would you like to take a shot at that, or just submit a new
> version that doesn't include the change to the return value?
>
> Thanks,
>
> Paolo
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQEcBAEBCAAGBQJVHCGwAAoJEL/70l94x66DuMcH/1Sb013BWNwot+I0jhMEr7jF
> WgaXQgIAVlxtrzspuE4cXF/TszCie/G1cGlF2oLP+GkJVivbAJFqJYNZcNDfE2Ti
> OdfhyUCgYMhOYUG81IXIbwXlGca/RhuDW74+B+tEL9dnoissI4l5JWS+k4bWK3On
> Pgu3gmIcLtQKTxUeDi93K25OAZNJJ9mLmf8CF4FAOUv3C5T4SN5tSkSEqQSB52by
> zKcM4+cXg5fzYH5OLo6d2SEUqsUHoEVh75VKLb38vAYm3GuNcuVoMEnlSELc3nTo
> A8Zrmc7swjZXqaZ6iRaXz7seiJ69XrfNWGqY+s0rGZoUK3z8/LwGSpDgmAY3pVQ=
> =Goct
> -----END PGP SIGNATURE-----