[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] cat: add -m option



On Mon, Jun 23, 2014 at 03:03:24PM +0200, Pino Toscano wrote:
> On Monday 23 June 2014 12:29:07 Richard W.M. Jones wrote:
> > On Mon, Jun 23, 2014 at 01:00:11PM +0200, Pino Toscano wrote:
> > >  static int
> > > 
> > > +do_cat_simple (int argc, char *argv[])
> > > +{
> > > +  unsigned errors = 0;
> > > +  int i;
> > > +
> > > +  for (i = 0; i < argc; ++i) {
> > > +    if (guestfs_download (g, argv[i], "/dev/stdout") == -1)
> > > +      errors++;
> > > +  }
> > > +
> > > +  return errors == 0 ? 0 : -1;
> > > +}
> > 
> > The problem with this function is two-fold:
> > 
> >  - It disables Windows path support if using the -m option, which
> >    seems a bit arbitrary.
> 
> I'm not sure: if I tell to mount an arbitrary partition/mountpoint, how 
> can libguestfs detect it's from a Windows guest?

I see, that makes sense.

> The main use cases I saw for this (and the other implementations for 
> virt-ls and virt-edit) were
> - handle partitions/disks which are parts of a guest (say the separate
>   /var in another disk)
> - handle data-only partitions (say the ones you put backups, etc)
> 
> So even if their filesystem is NTFS or something for Windows, there 
> should be no need for handling Windows paths (with the drive letter), 
> no?
> 
> >  - It duplicates parts of the original do_cat function.
> > 
> > What's wrong with modifying do_cat so it checks the global inspector
> > variable?
> 
> My first version of this patch was like that, then I thought it would 
> have been better to have inspection and no-inspection separated to avoid 
> cluttering a single function. I can put them together if wanted.

My spidey sense starts to tingle when I see duplicated code ...

Whichever is going to be simpler, but I'd still prefer to avoid
duplicated code as seen in this version of the patch.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]