On Wed, Aug 22, 2012 at 02:41:55PM +0800, Wanlong Gao wrote:
Add a new api xfs_repair for repairing an XFS filesystem. + ADD_ARG (argv, i, "xfs_repair"); +
As a matter of style, it might be better to write: if (optargs_bitmask & GUESTFS_XFS_REPAIR_FORCELOGZERO_BITMASK) { if (forcelogzero) ADD_ARG (argv, i, "-L"); } It just keeps all the logic in a single place.
+ if (!(optargs_bitmask & GUESTFS_XFS_REPAIR_IMGFILE_BITMASK)) + imgfile = 0;
This xfs_repair -f option is annoying! Also the way you've defined the "device" parameter (as type Device) means it won't work -- the caller would never be able to use a non-device as a parameter. Instead, can we check for the input being file or device and add the option automatically? It should be sufficient to change the code to something like: ... Dev_or_path "device" ... if (STRPREFIX (device, "/dev/")) is_device = 1; if (!is_device) { /* do the sysroot adjustment, and add -f parameter */ } else { /* just add device parameter */ }
+ if (!(optargs_bitmask & GUESTFS_XFS_REPAIR_NOMODIFY_BITMASK)) + nomodify = 0;
OK, but, the return value from xfs_repair is effectively thrown away, so this parameter would not be useful. I think you need to use 'commandrv' and do something useful with the return code. Look at what the fsck APIs do ...
+ { defaults with + name = "xfs_repair"; + style = RErr, [Device "device"], [OBool "imgfile"; OBool "forcelogzero"; OBool "dangerous"; OBool "nomodify"; OBool "noprefetch"; OBool "forcegeometry"; OInt64 "maxmem"; OInt64 "ihashsize"; OInt64 "bhashsize"; OInt64 "agstride"; OString "logdev"; OString "rtdev"]; + proc_nr = Some 350; + optional = Some "xfs"; + tests = [ + InitEmpty, IfAvailable "xfs", TestRun ( + [["part_disk"; "/dev/sda"; "mbr"]; + ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""]; + ["xfs_repair"; "/dev/sda1"; ""; ""; ""; "true"; ""; ""; ""; ""; ""; ""; "NOARG"; "NOARG"] + ])
A bit light on testing, but the API seems reasonable. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v