Re: [libvirt] [PATCH 2/4] Avoid create/unlink with block devs used for QEMU save
On 04/21/2010 10:56 AM, Daniel P. Berrange wrote:
It is possible to use block devices with domain save/restore. Upon
failure QEMU unlinks the path being saved to. This isn't good when
it is a block device !
+ struct stat sb;
+ int is_bdev = 0;
Should this be bool instead of int?
+ /* path might be a pre-existing block dev, in which case
+ * we need to skip the create step, and also avoid unlink
+ * in the failure case */
+ if (stat(path, &sb) < 0) {
+ if (errno != ENOENT) {
+ virReportSystemError(errno, _("unable to access %s"), path);
+ goto endjob;
+ } else {
+ is_bdev = 0;
+ }
+ } else {
+ is_bdev = S_ISBLK(sb.st_mode);
although if you use bool, you'd have to explicitly compare against 0
here, based on the gnulib restraints on stdbool.h. Also, what about
other non-regular files? A directory will fail on the open, but what
about named fifos (on the other hand, is anyone crazy enough to try a
non-seekable file as the file to open?). So I'm wondering if it's
better to check for S_ISREG, changing is_bdev to is_reg, and changing
the cleanup logic to only attempt unlink() if is_reg, rather than
skipping if is_bdev.
+ }
+
+
/* Setup hook data needed by virFileOperation hook function */
hdata.dom = dom;
hdata.path = path;
@@ -4849,7 +4866,8 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
/* Write header to file, followed by XML */
/* First try creating the file as root */
- if ((rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
+ if (!is_bdev &&
+ (rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
This is racy. You stat() prior to open()ing. Wouldn't it be better to
open(), then fstat()?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)