ä 2011å02æ09æ 00:52, Eric Blake åé:
On 02/08/2011 03:30 AM, Osier Yang wrote:
@@ -1739,11 +1746,23 @@ if test "$with_storage_disk" = "yes" ||
with_storage_disk=yes
fi
+ if test "$DMSETUP_FOUND" = "no" ; then
+ if test "$with_storage_disk" = "yes" ; then
+ AC_MSG_ERROR([We need dmsetup for disk storage driver])
+ else
+ with_storage_disk=no
+ fi
+ else
+ with_storage_disk=yes
+ fi
Logic is still not right here. You have two separate if clauses that
both try to convert with_storage_disk=check into
with_storage_disk=yes/no, but after the first block has run, the second
block never knwos that with_storage_disk used to be check.
It needs to be something like:
if test "$with_storage_disk" = "yes"&&
test "$PARTED_FOUND:$DMSETUP_FOUND" != "yes:yes"; then
AC_MSG_ERROR([Need both parted and dmsetup for disk storage driver])
fi
if test "$with_storage_disk" = "check"; then
if test "$PARTED_FOUND:$DMSETUP_FOUND" != "yes:yes"; then
with_storage_disk=no
else
with_storage_disk=yes
fi
fi
That is, if with_storage_disk is yes, then insist that both programs be
found; if with_storage_disk is check, then set with_storage_disk exactly
once based on the presence of both programs.
Oops, yes, didn't pay much attention here. :-)
So we'll need to see a v3.
@@ -895,6 +896,8 @@ virSetCloseExec;
virSetNonBlock;
virSetUIDGID;
virSkipSpaces;
+virStrcpy;
+virStrncpy;
virStrToDouble;
virStrToLong_i;
virStrToLong_l;
@@ -902,8 +905,6 @@ virStrToLong_ll;
virStrToLong_ui;
virStrToLong_ul;
virStrToLong_ull;
-virStrcpy;
-virStrncpy;
Hmm, you must have used emacs sorting while in LC_ALL=en_US.UTF-8
instead of LC_ALL=C. Personally, I leave LC_ALL undefined,
LANG=en_US.UTF-8, and LC_COLLATE=C, to get byte-value sorting
everywhere. I'm not sure if it makes sense to keep these hunks.
@@ -660,38 +662,46 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED,
srcname = basename(pool->def->source.devices[0].path);
DEBUG("devname=%s, srcname=%s", devname, srcname);
- if (!STRPREFIX(devname, srcname)) {
+ if (!virIsDevMapperDevice(devpath)&& !STRPREFIX(devname, srcname)) {
Let's swap the order of the conditionals. STRPREFIX is potentially
faster than virIsDevMapperDevice (since the latter calls stat() directly
and who knows what other syscalls in dm_is_dm_major).
yes, carefully reviewing, will update.
virStorageReportError(VIR_ERR_INTERNAL_ERROR,
_("Volume path '%s' did not start with parent "
"pool source device name."), devname);
goto cleanup;
}
- part_num = devname + strlen(srcname);
+ if (!virIsDevMapperDevice(devpath)) {
And, since it involves syscalls, can we cache the answer in a local bool
variable, rather than calling it twice?
will update
+ } else {
+ cmd = virCommandNew(DMSETUP);
+ virCommandAddArgList(cmd, "remove", "--force", devpath, NULL);
If you want, you can use virCommandNewArgList instead of
virCommandNew/virCommandAddArgList.
It's more compact, so, okay.
+bool
+virIsDevMapperDevice(const char *devname)
+{
+ struct stat buf;
+
+ if (devname&&
+ !stat(devname,&buf)&&
+ dm_is_dm_major(major(buf.st_rdev)))
major() is not required by POSIX; are we guaranteed that this will
compile everywhere? Also, are we sure that st_rdev is defined on all
file types, or should we add an extra condition that S_ISBLK(buf.st_mode)?
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list