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

Re: [libvirt] [PATCH v3 14/18] blockcopy: tweak how rebase calls into copy



On 09/05/2014 08:09 AM, Peter Krempa wrote:
> On 08/31/14 06:02, Eric Blake wrote:
>> In order to implement the new virDomainBlockCopy, the existing
>> block copy internal implementation needs to be adjusted.  The
>> new function will parse XML into a storage source, and parse
>> typed parameters into integers, then call into the same common
>> backend.  For now, it's easier to keep the same implementation
>> limits that only local file destinations are suported, but now
>> the check needs to be explicit.
>>

>>
>>  /* bandwidth in bytes/s */
>>  static int
>> -qemuDomainBlockCopy(virDomainObjPtr vm,
>> -                    virConnectPtr conn,
>> -                    const char *path,
>> -                    const char *dest, const char *format,
>> -                    unsigned long long bandwidth, unsigned int flags)
> 
> You should mention that this function consumes @dest.
> 
>> +qemuDomainBlockCopyCommon(virDomainObjPtr vm,

>>
>> -    if (VIR_ALLOC(mirror) < 0)
>> +    if (!(mirror = virStorageSourceCopy(dest, false)))
> 
> Also you won't need to copy it then.
> 

> 
> ACK with the docs added. The tweaks of the @dest handling are not necessary.
> 

How about the following interdiff?

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 7026a18..26e0237 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -15349,12 +15349,13 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom,
const char *path,


 /* bandwidth in bytes/s.  Caller must lock vm beforehand, and not
- * access it afterwards.  */
+ * access it afterwards; likewise, caller must not access mirror
+ * afterwards.  */
 static int
 qemuDomainBlockCopyCommon(virDomainObjPtr vm,
                           virConnectPtr conn,
                           const char *path,
-                          virStorageSourcePtr dest,
+                          virStorageSourcePtr mirror,
                           unsigned long long bandwidth,
                           unsigned int flags)
 {
@@ -15366,10 +15367,9 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
     int idx;
     struct stat st;
     bool need_unlink = false;
-    virStorageSourcePtr mirror = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
     const char *format = NULL;
-    int desttype = virStorageSourceGetActualType(dest);
+    int desttype = virStorageSourceGetActualType(mirror);

     /* Preliminaries: find the disk we are editing, sanity checks */
     virCheckFlags(VIR_DOMAIN_BLOCK_COPY_SHALLOW |
@@ -15419,7 +15419,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
         goto endjob;

     if ((flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW) &&
-        dest->format == VIR_STORAGE_FILE_RAW &&
+        mirror->format == VIR_STORAGE_FILE_RAW &&
         disk->src->backingStore->path) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("disk '%s' has backing file, so raw shallow copy "
@@ -15430,20 +15430,20 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,

     /* Prepare the destination file.  */
     /* XXX Allow non-file mirror destinations */
-    if (!virStorageSourceIsLocalStorage(dest)) {
+    if (!virStorageSourceIsLocalStorage(mirror)) {
         virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
                        _("non-file destination not supported yet"));
     }
-    if (stat(dest->path, &st) < 0) {
+    if (stat(mirror->path, &st) < 0) {
         if (errno != ENOENT) {
             virReportSystemError(errno, _("unable to stat for disk %s:
%s"),
-                                 disk->dst, dest->path);
+                                 disk->dst, mirror->path);
             goto endjob;
         } else if (flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT ||
                    desttype == VIR_STORAGE_TYPE_BLOCK) {
             virReportSystemError(errno,
                                  _("missing destination file for disk
%s: %s"),
-                                 disk->dst, dest->path);
+                                 disk->dst, mirror->path);
             goto endjob;
         }
     } else if (!S_ISBLK(st.st_mode)) {
@@ -15451,22 +15451,19 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("external destination file for disk %s
already "
                              "exists and is not a block device: %s"),
-                           disk->dst, dest->path);
+                           disk->dst, mirror->path);
             goto endjob;
         }
         if (desttype == VIR_STORAGE_TYPE_BLOCK) {
             virReportError(VIR_ERR_INVALID_ARG,
                            _("blockdev flag requested for disk %s, but
file "
                              "'%s' is not a block device"),
-                           disk->dst, dest->path);
+                           disk->dst, mirror->path);
             goto endjob;
         }
     }

-    if (!(mirror = virStorageSourceCopy(dest, false)))
-        goto endjob;
-
-    if (!dest->format) {
+    if (!mirror->format) {
         if (!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT)) {
             mirror->format = disk->src->format;
         } else {
@@ -15474,14 +15471,14 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
              * can also pass the RAW flag or use XML to tell us the format.
              * So if we get here, we assume it is safe for us to probe the
              * format from the file that we will be using.  */
-            mirror->format = virStorageFileProbeFormat(dest->path,
cfg->user,
+            mirror->format = virStorageFileProbeFormat(mirror->path,
cfg->user,
                                                        cfg->group);
         }
     }

     /* pre-create the image file */
     if (!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT)) {
-        int fd = qemuOpenFile(driver, vm, dest->path,
+        int fd = qemuOpenFile(driver, vm, mirror->path,
                               O_WRONLY | O_TRUNC | O_CREAT,
                               &need_unlink, NULL);
         if (fd < 0)
@@ -15504,7 +15501,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,

     /* Actually start the mirroring */
     qemuDomainObjEnterMonitor(driver, vm);
-    ret = qemuMonitorDriveMirror(priv->mon, device, dest->path, format,
+    ret = qemuMonitorDriveMirror(priv->mon, device, mirror->path, format,
                                  bandwidth, flags);
     virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0);
     qemuDomainObjExitMonitor(driver, vm);
@@ -15525,8 +15522,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
                  vm->def->name);

  endjob:
-    if (need_unlink && unlink(dest->path))
-        VIR_WARN("unable to unlink just-created %s", dest->path);
+    if (need_unlink && unlink(mirror->path))
+        VIR_WARN("unable to unlink just-created %s", mirror->path);
     virStorageSourceFree(mirror);
     if (!qemuDomainObjEndJob(driver, vm))
         vm = NULL;
@@ -15603,6 +15600,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const
char *path, const char *base,
     ret = qemuDomainBlockCopyCommon(vm, dom->conn, path, dest,
                                     bandwidth, flags);
     vm = NULL;
+    dest = NULL;

  cleanup:
     if (vm)


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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