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

Re: [libvirt] [PATCH v3 1/2] helper of copy-storage-* features



在 2012-11-09五的 07:27 -0700,Eric Blake写道:
> On 11/08/2012 10:26 PM, li guang wrote:
> 
> >>> +{
> >>> +    char *tmp_dir = NULL, *outbuf = NULL;
> >>> +    char *img_tool = driver->qemuImgBinary;
> >>
> >> Don't grab this field directly.  Instead, use qemuFindQemuImgBinary(driver).
> > 
> > driver->qemuImgBinary is useful, I've initiated it.
> > why don't we use it?
> 
> In the order your series was written, driver->qemuImgBinary may be
> uninitialized.  If you swap the order and put patch 2/2 first, then yes,
> you could use this field directly instead of calling
> qemuFindQemuImgBinary.  You should always try to order your patches so
> that no patch is broken until later patches in the series have been applied.
> 
> >>> +        if (gen_del) {
> >>> +            cmd = virCommandNewArgList(img_tool, "create", "-f",
> >>> +                                       def->disks[i]->driverType, def->disks[i]->src, NULL);
> >>> +            if (STREQ(def->disks[i]->driverType, "qcow2") ||
> >>> +                STREQ(def->disks[i]->driverType, "qcow"))
> >>> +                virCommandAddArgFormat(cmd, "%lluK", 0xffffffffff);
> >>
> >> Ouch.  You should not be passing in random sizes to qemu - if you need
> >> to know the source size in order to create a file on the destination
> >> side, then we HAVE to modify the migration cookie in order to pass
> >> sizing information when the new flag is present.
> >>
> > 
> > passing by cookie seems a little rough, 
> > these sizes are not random, they're 
> > the max size of their format.
> > does setting max size hurt?
> 
> Yes, because it is wrong.
> 
> > the disk images are sparse files,
> 
> So?  

real size of disk image will not take much disk space.

> A user that calls virsh vol-info on the file on the destination
> side should not get infinite size, but the same size as they would get
> on the source.  Just because it doesn't (yet) occupy the entire disk
> doesn't mean that we should size it so large that the file can grow
> without effective bounds and cause ENOSPC.

um, vol-info is a problem, 
though it seems odd for me.
can we add size field to _virDomainDiskDef?
if be allowed, life will be easier.

> 
> > and after qemu migration finished, size will be right.
> 
> If qemu changes the size after migration, then why can't it change from
> 0 to the proper size, rather than from max to the proper size?

I mean the disk size not virtual size.

> 

-- 
li guang  lig fnst cn fujitsu com
linux kernel team at FNST, china



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