openkylin
/
libvirt
mirror of https://gitee.com/openkylin/libvirt.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

snapshot: also support disks by path 

I got confused when 'virsh domblkinfo dom disk' required the
path to a disk (which can be ambiguous, since a single file
can back multiple disks), rather than the unambiguous target
device name that I was using in disk snapshots.  So, in true
developer fashion, I went for the best of both worlds - all
interfaces that operate on a disk (aka block) now accept
either the target name or the unambiguous path to the backing
file used by the disk.

* src/conf/domain_conf.h (virDomainDiskIndexByName): Add
parameter.
(virDomainDiskPathByName): New prototype.
* src/libvirt_private.syms (domain_conf.h): Export it.
* src/conf/domain_conf.c (virDomainDiskIndexByName): Also allow
searching by path, and decide whether ambiguity is okay.
(virDomainDiskPathByName): New function.
(virDomainDiskRemoveByName, virDomainSnapshotAlignDisks): Update
callers.
* src/qemu/qemu_driver.c (qemudDomainBlockPeek)
(qemuDomainAttachDeviceConfig, qemuDomainUpdateDeviceConfig)
(qemuDomainGetBlockInfo, qemuDiskPathToAlias): Likewise.
* src/qemu/qemu_process.c (qemuProcessFindDomainDiskByPath):
Likewise.
* src/libxl/libxl_driver.c (libxlDomainAttachDeviceDiskLive)
(libxlDomainDetachDeviceDiskLive, libxlDomainAttachDeviceConfig)
(libxlDomainUpdateDeviceConfig): Likewise.
* src/uml/uml_driver.c (umlDomainBlockPeek): Likewise.
* src/xen/xend_internal.c (xenDaemonDomainBlockPeek): Likewise.
* docs/formatsnapshot.html.in: Update documentation.
* tools/virsh.pod (domblkstat, domblkinfo): Likewise.
* docs/schemas/domaincommon.rng (diskTarget): Tighten pattern on
disk targets.
* docs/schemas/domainsnapshot.rng (disksnapshot): Update to match.
* tests/domainsnapshotxml2xmlin/disk_snapshot.xml: Update test.
This commit is contained in:
Eric Blake 2011-08-19 20:38:36 -06:00
parent d6f6b2d194
commit 89b6284fd9
13 changed files with 160 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
docs/formatsnapshot.html.in
View File

@ -129,8 +129,9 @@
<dt><code>disk</code></dt>
<dd>This sub-element describes the snapshot properties of a
specific disk. The attribute <code>name</code> is
mandatory, and must match the <code>&lt;target
dev='name'/&gt;</code> of one of
mandatory, and must match either the <code>&lt;target
dev='name'/&gt;</code> or an unambiguous <code>&lt;source
file='name'/&gt;</code> of one of
the <a href="formatdomain.html#elementsDisks">disk
devices</a> specified for the domain at the time of the
snapshot. The attribute <code>snapshot</code> is
@ -203,7 +204,7 @@
&lt;domainsnapshot&gt;
&lt;description&gt;Snapshot of OS install and updates&lt;/description&gt;
&lt;disks&gt;
&lt;disk name='vda'&gt;
&lt;disk name='/path/to/old'&gt;
&lt;source file='/path/to/new'/&gt;
&lt;/disk&gt;
&lt;disk name='vdb' snapshot='no'/&gt;

7
docs/schemas/domaincommon.rng
View File

@ -770,10 +770,15 @@
</choice>
</element>
</define>
<define name="diskTarget">
<data type="string">
<param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+</param>
</data>
</define>
<define name="target">
<element name="target">
<attribute name="dev">
<ref name="deviceName"/>
<ref name="diskTarget"/>
</attribute>
<optional>
<attribute name="bus">

5
docs/schemas/domainsnapshot.rng
View File

@ -82,7 +82,10 @@
<define name='disksnapshot'>
<element name='disk'>
<attribute name='name'>
<ref name='deviceName'/>
<choice>
<ref name='diskTarget'/>
<ref name='absFilePath'/>
</choice>
</attribute>
<choice>
<attribute name='snapshot'>

57
src/conf/domain_conf.c
View File

@ -5811,17 +5811,44 @@ virDomainChrTargetTypeToString(int deviceType,
return type;
}
int virDomainDiskIndexByName(virDomainDefPtr def, const char *name)
int
virDomainDiskIndexByName(virDomainDefPtr def, const char *name,
bool allow_ambiguous)
{
virDomainDiskDefPtr vdisk;
int i;
int candidate = -1;
/* We prefer the <target dev='name'/> name (it's shorter, required
* for all disks, and should be unambiguous), but also support
* <source file='name'/> (if unambiguous). Assume dst if there is
* no leading slash, source name otherwise. */
for (i = 0; i < def->ndisks; i++) {
vdisk = def->disks[i];
if (STREQ(vdisk->dst, name))
return i;
if (*name != '/') {
if (STREQ(vdisk->dst, name))
return i;
} else if (vdisk->src &&
STREQ(vdisk->src, name)) {
if (allow_ambiguous)
return i;
if (candidate >= 0)
return -1;
candidate = i;
}
}
return -1;
return candidate;
}
/* Return the path to a disk image if a string identifies at least one
* disk belonging to the domain (both device strings 'vda' and paths
* '/path/to/file' are converted into '/path/to/file'). */
const char *
virDomainDiskPathByName(virDomainDefPtr def, const char *name)
{
int i = virDomainDiskIndexByName(def, name, true);
return i < 0 ? NULL : def->disks[i]->src;
}
int virDomainDiskInsert(virDomainDefPtr def,
@ -5897,7 +5924,7 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i)
int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name)
{
int i = virDomainDiskIndexByName(def, name);
int i = virDomainDiskIndexByName(def, name, false);
if (i < 0)
return -1;
virDomainDiskRemove(def, i);
@ -11651,11 +11678,12 @@ disksorter(const void *a, const void *b)
/* Align def->disks to def->domain. Sort the list of def->disks,
* filling in any missing disks or snapshot state defaults given by
* the domain, with a fallback to a passed in default. Issue an error
* and return -1 if any def->disks[n]->name appears more than once or
* does not map to dom->disks. If require_match, also require that
* existing def->disks snapshot states do not override explicit
* def->dom settings. */
* the domain, with a fallback to a passed in default. Convert paths
* to disk targets for uniformity. Issue an error and return -1 if
* any def->disks[n]->name appears more than once or does not map to
* dom->disks. If require_match, also require that existing
* def->disks snapshot states do not override explicit def->dom
* settings. */
int
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
int default_snapshot,
@ -11693,7 +11721,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
/* Double check requested disks. */
for (i = 0; i < def->ndisks; i++) {
virDomainSnapshotDiskDefPtr disk = &def->disks[i];
int idx = virDomainDiskIndexByName(def->dom, disk->name);
int idx = virDomainDiskIndexByName(def->dom, disk->name, false);
int disk_snapshot;
if (idx < 0) {
@ -11731,6 +11759,13 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
disk->file, disk->name);
goto cleanup;
}
if (STRNEQ(disk->name, def->dom->disks[idx]->dst)) {
VIR_FREE(disk->name);
if (!(disk->name = strdup(def->dom->disks[idx]->dst))) {
virReportOOMError();
goto cleanup;
}
}
}
/* Provide defaults for all remaining disks. */

4
src/conf/domain_conf.h
View File

@ -1648,7 +1648,9 @@ int virDomainVcpuPinAdd(virDomainDefPtr def,
int virDomainVcpuPinDel(virDomainDefPtr def, int vcpu);
int virDomainDiskIndexByName(virDomainDefPtr def, const char *name);
int virDomainDiskIndexByName(virDomainDefPtr def, const char *name,
bool allow_ambiguous);
const char *virDomainDiskPathByName(virDomainDefPtr, const char *name);
int virDomainDiskInsert(virDomainDefPtr def,
virDomainDiskDefPtr disk);
void virDomainDiskInsertPreAlloced(virDomainDefPtr def,

1
src/libvirt_private.syms
View File

@ -287,6 +287,7 @@ virDomainDiskInsert;
virDomainDiskInsertPreAlloced;
virDomainDiskIoTypeFromString;
virDomainDiskIoTypeToString;
virDomainDiskPathByName;
virDomainDiskRemove;
virDomainDiskRemoveByName;
virDomainDiskSnapshotTypeFromString;

11
src/libxl/libxl_driver.c
View File

@ -2929,7 +2929,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) {
if (virDomainDiskIndexByName(vm->def, l_disk->dst) >= 0) {
if (virDomainDiskIndexByName(vm->def, l_disk->dst, true) >= 0) {
libxlError(VIR_ERR_OPERATION_FAILED,
_("target %s already exists"), l_disk->dst);
goto cleanup;
@ -2991,7 +2991,8 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_XEN) {
if ((i = virDomainDiskIndexByName(vm->def,
dev->data.disk->dst)) < 0) {
dev->data.disk->dst,
false)) < 0) {
libxlError(VIR_ERR_OPERATION_FAILED,
_("disk %s not found"), dev->data.disk->dst);
goto cleanup;
@ -3061,7 +3062,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
disk = dev->data.disk;
if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) {
if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) {
libxlError(VIR_ERR_INVALID_ARG,
_("target %s already exists."), disk->dst);
return -1;
@ -3172,9 +3173,9 @@ libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
disk = dev->data.disk;
if ((i = virDomainDiskIndexByName(vmdef, disk->dst)) < 0) {
if ((i = virDomainDiskIndexByName(vmdef, disk->dst, false)) < 0) {
libxlError(VIR_ERR_INVALID_ARG,
_("target %s doesn't exists."), disk->dst);
_("target %s doesn't exist."), disk->dst);
goto cleanup;
}
orig = vmdef->disks[i];

112
src/qemu/qemu_driver.c
View File

@ -5495,7 +5495,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
disk = dev->data.disk;
if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) {
if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) {
qemuReportError(VIR_ERR_INVALID_ARG,
_("target %s already exists."), disk->dst);
return -1;
@ -5613,10 +5613,10 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
disk = dev->data.disk;
pos = virDomainDiskIndexByName(vmdef, disk->dst);
pos = virDomainDiskIndexByName(vmdef, disk->dst, false);
if (pos < 0) {
qemuReportError(VIR_ERR_INVALID_ARG,
_("target %s doesn't exists."), disk->dst);
_("target %s doesn't exist."), disk->dst);
return -1;
}
orig = vmdef->disks[pos];
@ -7355,7 +7355,8 @@ qemudDomainBlockPeek (virDomainPtr dom,
{
struct qemud_driver *driver = dom->conn->privateData;
virDomainObjPtr vm;
int fd = -1, ret = -1, i;
int fd = -1, ret = -1;
const char *actual;
virCheckFlags(0, -1);
@ -7377,41 +7378,34 @@ qemudDomainBlockPeek (virDomainPtr dom,
goto cleanup;
}
/* Check the path belongs to this domain. */
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (vm->def->disks[i]->src != NULL &&
STREQ (vm->def->disks[i]->src, path)) {
ret = 0;
break;
}
}
if (ret == 0) {
ret = -1;
/* The path is correct, now try to open it and get its size. */
fd = open (path, O_RDONLY);
if (fd == -1) {
virReportSystemError(errno,
_("%s: failed to open"), path);
goto cleanup;
}
/* Seek and read. */
/* NB. Because we configure with AC_SYS_LARGEFILE, off_t should
* be 64 bits on all platforms.
*/
if (lseek (fd, offset, SEEK_SET) == (off_t) -1 ||
saferead (fd, buffer, size) == (ssize_t) -1) {
virReportSystemError(errno,
_("%s: failed to seek or read"), path);
goto cleanup;
}
ret = 0;
} else {
/* Check the path belongs to this domain. */
if (!(actual = virDomainDiskPathByName(vm->def, path))) {
qemuReportError(VIR_ERR_INVALID_ARG,
"%s", _("invalid path"));
_("invalid path '%s'"), path);
goto cleanup;
}
path = actual;
/* The path is correct, now try to open it and get its size. */
fd = open(path, O_RDONLY);
if (fd == -1) {
virReportSystemError(errno,
_("%s: failed to open"), path);
goto cleanup;
}
/* Seek and read. */
/* NB. Because we configure with AC_SYS_LARGEFILE, off_t should
* be 64 bits on all platforms.
*/
if (lseek(fd, offset, SEEK_SET) == (off_t) -1 ||
saferead(fd, buffer, size) == (ssize_t) -1) {
virReportSystemError(errno,
_("%s: failed to seek or read"), path);
goto cleanup;
}
ret = 0;
cleanup:
VIR_FORCE_CLOSE(fd);
@ -7491,7 +7485,7 @@ qemudDomainMemoryPeek (virDomainPtr dom,
qemuDomainObjExitMonitor(driver, vm);
/* Read the memory file into buffer. */
if (saferead (fd, buffer, size) == (ssize_t) -1) {
if (saferead(fd, buffer, size) == (ssize_t) -1) {
virReportSystemError(errno,
_("failed to read temporary file "
"created with template %s"), tmp);
@ -7527,8 +7521,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
virStorageFileMetadata *meta = NULL;
virDomainDiskDefPtr disk = NULL;
struct stat sb;
int i;
int format;
const char *actual;
virCheckFlags(0, -1);
@ -7550,22 +7544,15 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
}
/* Check the path belongs to this domain. */
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (vm->def->disks[i]->src != NULL &&
STREQ (vm->def->disks[i]->src, path)) {
disk = vm->def->disks[i];
break;
}
}
if (!disk) {
if (!(actual = virDomainDiskPathByName(vm->def, path))) {
qemuReportError(VIR_ERR_INVALID_ARG,
_("invalid path %s not assigned to domain"), path);
goto cleanup;
}
path = actual;
/* The path is correct, now try to open it and get its size. */
fd = open (path, O_RDONLY);
fd = open(path, O_RDONLY);
if (fd == -1) {
virReportSystemError(errno,
_("failed to open path '%s'"), path);
@ -7624,7 +7611,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
/* NB. Because we configure with AC_SYS_LARGEFILE, off_t should
* be 64 bits on all platforms.
*/
end = lseek (fd, 0, SEEK_END);
end = lseek(fd, 0, SEEK_END);
if (end == (off_t)-1) {
virReportSystemError(errno,
_("failed to seek to end of %s"), path);
@ -9831,23 +9818,26 @@ static const char *
qemuDiskPathToAlias(virDomainObjPtr vm, const char *path) {
int i;
char *ret = NULL;
virDomainDiskDefPtr disk;
for (i = 0 ; i < vm->def->ndisks ; i++) {
virDomainDiskDefPtr disk = vm->def->disks[i];
i = virDomainDiskIndexByName(vm->def, path, true);
if (i < 0)
goto cleanup;
if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK &&
disk->type != VIR_DOMAIN_DISK_TYPE_FILE)
continue;
disk = vm->def->disks[i];
if (disk->src != NULL && STREQ(disk->src, path)) {
if (virAsprintf(&ret, "drive-%s", disk->info.alias) < 0) {
virReportOOMError();
return NULL;
}
break;
if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK &&
disk->type != VIR_DOMAIN_DISK_TYPE_FILE)
goto cleanup;
if (disk->src) {
if (virAsprintf(&ret, "drive-%s", disk->info.alias) < 0) {
virReportOOMError();
return NULL;
}
}
cleanup:
if (!ret) {
qemuReportError(VIR_ERR_INVALID_ARG,
"%s", _("No device found for specified path"));

11
src/qemu/qemu_process.c
View File

@ -192,15 +192,10 @@ static virDomainDiskDefPtr
qemuProcessFindDomainDiskByPath(virDomainObjPtr vm,
const char *path)
{
int i;
int i = virDomainDiskIndexByName(vm->def, path, true);
for (i = 0; i < vm->def->ndisks; i++) {
virDomainDiskDefPtr disk;
disk = vm->def->disks[i];
if (disk->src != NULL && STREQ(disk->src, path))
return disk;
}
if (i >= 0)
return vm->def->disks[i];
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("no disk found with path %s"),

58
src/uml/uml_driver.c
View File

@ -2175,7 +2175,8 @@ umlDomainBlockPeek(virDomainPtr dom,
{
struct uml_driver *driver = dom->conn->privateData;
virDomainObjPtr vm;
int fd = -1, ret = -1, i;
int fd = -1, ret = -1;
const char *actual;
virCheckFlags(0, -1);
@ -2196,41 +2197,34 @@ umlDomainBlockPeek(virDomainPtr dom,
}
/* Check the path belongs to this domain. */
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (vm->def->disks[i]->src != NULL &&
STREQ (vm->def->disks[i]->src, path)) {
ret = 0;
break;
}
if (!(actual = virDomainDiskPathByName(vm->def, path))) {
umlReportError(VIR_ERR_INVALID_ARG,
_("invalid path '%s'"), path);
goto cleanup;
}
path = actual;
/* The path is correct, now try to open it and get its size. */
fd = open(path, O_RDONLY);
if (fd == -1) {
virReportSystemError(errno,
_("cannot open %s"), path);
goto cleanup;
}
if (ret == 0) {
ret = -1;
/* The path is correct, now try to open it and get its size. */
fd = open (path, O_RDONLY);
if (fd == -1) {
virReportSystemError(errno,
_("cannot open %s"), path);
goto cleanup;
}
/* Seek and read. */
/* NB. Because we configure with AC_SYS_LARGEFILE, off_t should
* be 64 bits on all platforms.
*/
if (lseek (fd, offset, SEEK_SET) == (off_t) -1 ||
saferead (fd, buffer, size) == (ssize_t) -1) {
virReportSystemError(errno,
_("cannot read %s"), path);
goto cleanup;
}
ret = 0;
} else {
umlReportError(VIR_ERR_INVALID_ARG, "%s",
_("invalid path"));
/* Seek and read. */
/* NB. Because we configure with AC_SYS_LARGEFILE, off_t should
* be 64 bits on all platforms.
*/
if (lseek(fd, offset, SEEK_SET) == (off_t) -1 ||
saferead(fd, buffer, size) == (ssize_t) -1) {
virReportSystemError(errno,
_("cannot read %s"), path);
goto cleanup;
}
ret = 0;
cleanup:
VIR_FORCE_CLOSE(fd);
if (vm)

12
src/xen/xend_internal.c
View File

@ -3849,11 +3849,11 @@ xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path,
xenUnifiedPrivatePtr priv;
struct sexpr *root = NULL;
int fd = -1, ret = -1;
int found = 0, i;
virDomainDefPtr def;
int id;
char * tty;
int vncport;
const char *actual;
priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
@ -3889,18 +3889,12 @@ xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path,
vncport)))
goto cleanup;
for (i = 0 ; i < def->ndisks ; i++) {
if (def->disks[i]->src &&
STREQ(def->disks[i]->src, path)) {
found = 1;
break;
}
}
if (!found) {
if (!(actual = virDomainDiskPathByName(def, path))) {
virXendError(VIR_ERR_INVALID_ARG,
_("%s: invalid path"), path);
goto cleanup;
}
path = actual;
/* The path is correct, now try to open it and get its size. */
fd = open (path, O_RDONLY);

2
tests/domainsnapshotxml2xmlin/disk_snapshot.xml
View File

@ -2,7 +2,7 @@
<name>my snap name</name>
<description>!@#$%^</description>
<disks>
<disk name='hda'/>
<disk name='/dev/HostVG/QEMUGuest1'/>
<disk name='hdb' snapshot='no'/>
<disk name='hdc' snapshot='internal'/>
<disk name='hdd' snapshot='external'>

8
tools/virsh.pod
View File

@ -503,7 +503,9 @@ snapshot metadata with B<snapshot-create>.
=item B<domblkstat> I<domain> I<block-device>
Get device block stats for a running domain.
Get device block stats for a running domain. A I<block-device> corresponds
to a unique target name (<target dev='name'/>) or source file (<source
file='name'/>) for one of the disk devices attached to I<domain>.
=item B<domifstat> I<domain> I<interface-device>
@ -515,7 +517,9 @@ Get memory stats for a running domain.
=item B<domblkinfo> I<domain> I<block-device>
Get block device size info for a domain.
Get block device size info for a domain. A I<block-device> corresponds
to a unique target name (<target dev='name'/>) or source file (<source
file='name'/>) for one of the disk devices attached to I<domain>.
=item B<blockpull> I<domain> I<path> [I<bandwidth>]