Change in vdsm[master]: v2v: Convert VM from external source to Data Domain

nsoffer at redhat.com nsoffer at redhat.com
Sun Mar 29 13:45:26 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: v2v: Convert VM from external source to Data Domain
......................................................................


Patch Set 14:

(4 comments)

https://gerrit.ovirt.org/#/c/37509/14/vdsm/v2v.py
File vdsm/v2v.py:

Line 325:                 self._status_msg = ex.message
Line 326:                 self._abort()
Line 327:                 raise
Line 328:         finally:
Line 329:             self._delete_passwd_file()
If this raises OSError that we don't handle, prepared voluems will not be tear down.

The best way to fix this would be to catch and log such failures here to keep _delete_passwd_file more general.

We must do this since raising in finally will hide errors from the except block, which is very bad thing to do, since these errors are the real errors that we must keep.

So:

    try:
        self._delete_passwd_file():
    except Excepiton:
        logging.exception("Error deleting passwd file: %r", self._passwd_file)
Line 330:             self._teardown_volumes()
Line 331: 
Line 332:     def _import(self):
Line 333:         # TODO: use the process handling http://gerrit.ovirt.org/#/c/33909/


Line 338:                              env={'LIBGUESTFS_BACKEND': 'direct'})
Line 339: 
Line 340:         self._proc.blocking = True
Line 341:         self._watch_process_output()
Line 342: 
You should wait for the process before checking the return code:

    logging.debug("Job %r waiting for virt-v2v process", self._id)
    self._proc.wait(timeout)

We should abort and fail if it does not finish within reasonable timeout.

Since parsing was finished without an error (stdout was closed), this should not take too much time, but this requires testing with v2v.
Line 343:         if self._proc.returncode != 0:
Line 344:             raise V2VProcessError('Process failed exit-code: %r, stderr: %s' %
Line 345:                                   (self._proc.returncode,
Line 346:                                    self._proc.stderr.read(1024)))


Line 339: 
Line 340:         self._proc.blocking = True
Line 341:         self._watch_process_output()
Line 342: 
Line 343:         if self._proc.returncode != 0:
returncode may be None if the process did not exit yet. This is unlikely but possible. In such case, we would drop good vm we just imported successfully.
Line 344:             raise V2VProcessError('Process failed exit-code: %r, stderr: %s' %
Line 345:                                   (self._proc.returncode,
Line 346:                                    self._proc.stderr.read(1024)))
Line 347:         logging.info('Importing VM finish, JobId:', self._id)


Line 464:             self._prepared_volumes.append([drive])
Line 465: 
Line 466:         return self._extract_storage_path(self._prepared_volumes[0]['path'])
Line 467: 
Line 468:     def _extract_storage_path(self, path):
> Rename to _get_storage_domain_path() and access self._prepared_volumes[0] i
You can also move it out of the class and make it a module function, accepting a volume path and returning the storage domain path.

Such function can be moved later to storage/volume.py
Line 469:         '''
Line 470:         prepareImage returns /prefix/sdUUID/images/imgUUID/volUUID
Line 471:         we need storage domain absolute path so we go up 3 levels
Line 472:         '''


-- 
To view, visit https://gerrit.ovirt.org/37509
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I34bd86d5a87ea8c42113c4a732f87ddd4ceab9ea
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list