Re: [Libguestfs] [PATCH V2] NEW API: add new api xfs_info

On Thu, Jul 12, 2012 at 11:15:57AM +0800, Wanlong Gao wrote: > Add xfs_info to show the geometry of the xfs filesystem. > > Signed-off-by: Wanlong Gao <gaowanlong(a)cn.fujitsu.com&gt; > --- > > Hi Rich, > > This is the v2 version, please help reviewing. > Formated the output, but seems that there's also > something wrong. And I'll add self test in next > version. Also you'll need to change it for the new way that functions are defined in the generator (see previous email). Sorry, I can only do a quick review now as I'm having all the electricity turned off this morning. > Thanks, > Wanlong Gao > > > daemon/Makefile.am | 1 + > daemon/xfs.c | 228 +++++++++++++++++++++++++++++++++++++++++ > generator/generator_actions.ml | 6 ++ > po/POTFILES | 1 + > src/MAX_PROC_NR | 2 +- > 5 files changed, 237 insertions(+), 1 deletion(-) > create mode 100644 daemon/xfs.c > > diff --git a/daemon/Makefile.am b/daemon/Makefile.am > index 9e2a633..afe8874 100644 > --- a/daemon/Makefile.am > +++ b/daemon/Makefile.am > @@ -165,6 +165,7 @@ guestfsd_SOURCES = \ > utimens.c \ > wc.c \ > xattr.c \ > + xfs.c \ > zero.c \ > zerofree.c > guestfsd_LDADD = \ > diff --git a/daemon/xfs.c b/daemon/xfs.c > new file mode 100644 > index 0000000..ea3782b > --- /dev/null > +++ b/daemon/xfs.c > @@ -0,0 +1,228 @@ > +/* libguestfs - the guestfsd daemon > + * Copyright (C) 2012 Fujitsu Limited. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + */ > + > +#include <config.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <inttypes.h> > +#include <string.h> > +#include <unistd.h> > + > +#include "guestfs_protocol.h" > +#include "daemon.h" > +#include "c-ctype.h" > +#include "actions.h" > + > +int > +optgroup_xfs_available (void) > +{ > + return prog_exists ("mkfs.xfs"); > +} > + > +char ** > +do_xfs_info (const char *path) > +{ > + int r; > + int i, j; > + char *buf; > + char *out = NULL, *err = NULL; > + char **lines = NULL; > + char *line = NULL, *p = NULL, *s = NULL; > + char strbuf[BUFSIZ]; > + > + DECLARE_STRINGSBUF (ret); > + > + if (do_is_dir (path)) { > + buf = sysroot_path (path); > + if (!buf) { > + reply_with_perror ("malloc"); > + return NULL; > + } > + } else { > + buf = strdup(path); > + if (!buf) { > + reply_with_perror ("strdup"); > + return NULL; > + } > + } > + > + r = command (&out, &err, "xfs_info", buf, NULL); > + free (buf); > + if (r == -1) { > + reply_with_error ("%s", err); > + goto error; > + } > + > + lines = split_lines (out); > + if (lines == NULL) > + goto error; > + > + const char *md[] = {"meta-data", "isize", "agcount", "agsize", > + "sectsz", "attr", NULL}; > + const char *data[] = {"data", "bsize", "blocks", "imaxpct", > + "sunit", "swidth", NULL}; > + const char *name[] = {"naming", "bsize", "ascii-ci", NULL}; > + const char *log[] = {"log", "bsize", "blocks", "version", > + "sectsz", "sunit", "lazy-count", NULL}; > + const char *rt[] = {"realtime", "extsz", "blocks", "rtextents", NULL}; > + > + for (j = 0; j < 2; j++) { > + line = lines[j]; > + if (!line) > + goto error; > + s = line; > + for (i = 0; md[i] != NULL; i++) { > + p = strstr(s, md[i]); > + if (p) { > + while (*p != '=') p++; > + p++; > + s = p; > + while (*s != ' ' && *s != ',') s++; > + *s = '\0'; > + s++; > + if (i == 0) { > + if (add_string (&ret, md[i]) == -1 || > + add_string (&ret, p) == -1) goto error; > + } else { > + sprintf (strbuf, "%s.%s", md[0], md[i]); > + if (add_string (&ret, strbuf) == -1 || > + add_string (&ret, p) == -1) goto error; > + } > + } > + } > + } > + > + for (j = 2; j < 4; j++) { > + line = lines[j]; > + if (!line) > + goto error; > + s = line; > + for (i = 1; data[i] != NULL; i++) { > + p = strstr(s, data[i]); > + if (p) { > + while (*p != '=') p++; > + p++; > + s = p; > + while (*s != ' ' && *s != ',') s++; > + *s = '\0'; > + s++; > + sprintf(strbuf, "%s.%s", data[0], data[i]); > + if (add_string (&ret, strbuf) == -1 || > + add_string (&ret, p) == -1) goto error; > + } > + } > + } > + > + line = lines[4]; > + if (!line) > + goto error; > + s = line; > + for (i = 0; name[i] != NULL; i++) { > + p = strstr(s, name[i]); > + if (p) { > + while (*p != '=') p++; > + p++; > + if (i == 0) { > + while (*p != ' ') p++; p++; > + } > + s = p; > + while (*s != ' ' && *s != ',') s++; > + *s = '\0'; > + s++; > + if (i == 0) { > + sprintf(strbuf, "%s.%s", name[i], "version"); > + if (add_string (&ret, strbuf) == -1 || > + add_string (&ret, p) == -1) goto error; > + } else { > + sprintf(strbuf, "%s.%s", name[0], name[i]); > + if (add_string (&ret, strbuf) == -1 || > + add_string (&ret, p) == -1) goto error; > + } > + } > + } > + > + > + for (j = 5; j < 7; j++) { > + line = lines[j]; > + if (!line) > + goto error; > + s = line; > + for (i = 0; log[i] != NULL; i++) { > + p = strstr(s, log[i]); > + if (p) { > + while (*p != '=') p++; > + p++; > + s = p; > + while (*s != ' ' && *s != ',') s++; > + *s = '\0'; > + s++; > + if (i == 0) { > + if (add_string (&ret, log[i]) == -1 || > + add_string (&ret, p) == -1) goto error; > + } else { > + sprintf(strbuf, "%s.%s", log[0], log[i]); > + if (add_string (&ret, strbuf) == -1 || > + add_string (&ret, p) == -1) goto error; > + } > + } > + } > + } > + > + > + line = lines[7]; > + if (!line) > + goto error; > + s = line; > + for (i = 0; rt[i] != NULL; i++) { > + p = strstr(s, rt[i]); > + if (p) { > + while (*p != '=') p++; > + p++; > + s = p; > + while (*s != ' ' && *s != ',') s++; > + *s = '\0'; > + s++; > + if (i == 0) { > + if (add_string (&ret, rt[i]) == -1 || > + add_string (&ret, p) == -1) goto error; > + } else { > + sprintf(strbuf, "%s.%s", rt[0], rt[i]); > + if (add_string (&ret, strbuf) == -1 || > + add_string (&ret, p) == -1) goto error; > + } > + } > + } > + > + free (out); > + free (err); > + free_strings (lines); > + > + if (end_stringsbuf (&ret) == -1) > + return NULL; > + > + return ret.argv; > + > +error: > + free (out); > + free (err); > + free_strings (lines); > + if (ret.argv != NULL) > + free_stringslen (ret.argv, ret.size); > + return NULL; > +} It seems like a reasonable approach given the complex and frankly obscure output of the xfs_info command. Did you have a look at the sources of XFS to see if the format is fixed or can change? It might also be worth considering using an 'RStruct' instead of 'RHashtable', although it mainly depends on whether the output can change in future or not.

> diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml > index 5baa9b2..e4cbee0 100644 > --- a/generator/generator_actions.ml > +++ b/generator/generator_actions.ml > @@ -7374,6 +7374,12 @@ be returned if you called C<guestfs_list_devices>. > To find out the maximum number of devices that could be added, > call C<guestfs_max_disks>."); > > + ("xfs_info", (RHashtable "info", [String "path"], []), 337, [Optional "xfs"], > + [], > + "print out the geometry of the filesystem", > + "\ > +Thie function can print out the geometry of an mounted XFS filesystem."); > + > ] > > let all_functions = non_daemon_functions @ daemon_functions > diff --git a/po/POTFILES b/po/POTFILES > index 747b341..8217314 100644 > --- a/po/POTFILES > +++ b/po/POTFILES > @@ -84,6 +84,7 @@ daemon/upload.c > daemon/utimens.c > daemon/wc.c > daemon/xattr.c > +daemon/xfs.c > daemon/zero.c > daemon/zerofree.c > df/df.c > diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR > index e64f24d..f59a90f 100644 > --- a/src/MAX_PROC_NR > +++ b/src/MAX_PROC_NR > @@ -1 +1 @@ > -336 > +337 > -- > 1.7.11.1.165.g299666c Rich.