--- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_monitor_json.c | 17 +++++++++++++++++ tests/qemumonitorjsontest.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 13c0372..ea57111 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -337,6 +337,7 @@ struct qemuDomainDiskInfo { bool tray_open; bool empty; int io_status; + unsigned long long guest_size; }; typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3d0a120..7903b47 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1800,6 +1800,8 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, for (i = 0; i < virJSONValueArraySize(devices); i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); + virJSONValuePtr inserted; + virJSONValuePtr image; struct qemuDomainDiskInfo *info; const char *thisdev; const char *status; @@ -1854,6 +1856,21 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, if (info->io_status < 0) goto cleanup; } + + if ((inserted = virJSONValueObjectGetObject(dev, "inserted"))) { + if (!(image = virJSONValueObjectGetObject(inserted, "image"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read 'inserted/image' value")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(image, "virtual-size", + &info->guest_size) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot read 'inserted/image/virtual-size' value")); + goto cleanup; + } + } } ret = 0; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index d3ec3b1..d1922fd 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -50,6 +50,23 @@ const char *queryBlockReply = " \"locked\": false," " \"removable\": false," " \"inserted\": {" +" \"image\": {" +" \"virtual-size\": 68719476736," +" \"filename\": \"/home/zippy/work/tmp/gentoo.qcow2\"," +" \"cluster-size\": 65536," +" \"format\": \"qcow2\"," +" \"actual-size\": 156901376," +" \"format-specific\": {" +" \"type\": \"qcow2\"," +" \"data\": {" +" \"compat\": \"1.1\"," +" \"lazy-refcounts\": true," +" \"refcount-bits\": 16," +" \"corrupt\": false" +" }" +" }," +" \"dirty-flag\": false" +" }," " \"iops_rd\": 5," " \"iops_wr\": 6," " \"ro\": false," @@ -78,6 +95,13 @@ const char *queryBlockReply = " \"locked\": false," " \"removable\": false," " \"inserted\": {" +" \"image\": {" +" \"virtual-size\": 34359738368," +" \"filename\": \"/home/zippy/test.bin\"," +" \"format\": \"raw\"," +" \"actual-size\": 34359738368," +" \"dirty-flag\": false" +" }," " \"iops_rd\": 0," " \"iops_wr\": 0," " \"ro\": false," @@ -99,6 +123,13 @@ const char *queryBlockReply = " \"locked\": true," " \"removable\": true," " \"inserted\": {" +" \"image\": {" +" \"virtual-size\": 17179869184," +" \"filename\": \"/home/zippy/test.bin\"," +" \"format\": \"raw\"," +" \"actual-size\": 17179869184," +" \"dirty-flag\": false" +" }," " \"iops_rd\": 0," " \"iops_wr\": 0," " \"ro\": true," @@ -1413,6 +1444,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data) if (VIR_ALLOC(info) < 0) goto cleanup; + info->guest_size = 68719476736ULL; + if (virHashAddEntry(expectedBlockDevices, "virtio-disk0", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedBlockDevices hash table"); @@ -1422,6 +1455,8 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data) if (VIR_ALLOC(info) < 0) goto cleanup; + info->guest_size = 34359738368ULL; + if (virHashAddEntry(expectedBlockDevices, "virtio-disk1", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedBlockDevices hash table"); @@ -1434,6 +1469,7 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data) info->locked = true; info->removable = true; info->tray = true; + info->guest_size = 17179869184ULL; if (virHashAddEntry(expectedBlockDevices, "ide0-1-0", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 1.8.3.1

On 26.09.2016 23:07, John Ferlan wrote: I donno, does backing chain changes anything in this case? I need virtual size of top image only and anyway is it possible for images of backing chain to have different virtual sizes? It does not make much sense. qemuDomainGetStatsBlock (and more specifically qemuMonitorBlockStatsUpdateCapacity which is more suited to get just size) has rather inconviniet arguments more suited for many disks requests. I would not make function with such specific name provide additionally size info... I think it is perfectily fine to have many monitor commands that internally use same 'query-block' qemu command because this command is really profound) I doubt there can be 'inserted' without 'image', it doesn't make sense, inserted what?)) I guess qemuDomainGetStatsBlock ignores missing 'image' because it can afford it, while qemuMonitorJSONDiskNameLookup does not really ignores, if name is not found then function return error eventually. So if this is true, there should be no 'inserted' without 'image', then shame will go to qemu) Not really. Backing chain has nothing to do with backup, there is just no difference from backup POV. Nikolay

On 28.09.2016 00:53, John Ferlan wrote: Backup sees disk just like guest, block device nothing more, no any backing chain structure. AFAIK full pull backups as well as full/incremennal push backups are ready to use in qemu. It is a pity that other commands that helps organize pull backup process still has experimental status. One day they drop this x- and I doubt that things will changed very radically in comparison to what we have today. So if with the help of community this patch will be ironed out libvirt will get pull backups instantly after qemu be full ready. Nikolay

On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote: I haven't looked forward to patch 3/3 yet, but perhaps it'd be better in the "caller" that cares to : if (qemu*() < 0) { virErrorPtr err = virGetLastError(); if (err && err->code == VIR_ERR_OPERATION_INVALID) { /* Do something specific */ } } Returning -2 alters virDomainBlockJobAbort expected return values and well that's probably not a good idea since we don't know if some caller has said (if virDomainBlockJobAbort() == -1) instead of <= -1 John

Add API to start/stop exporting disks for a backup and add qemu implementation. The latter is not complete yet. At least backup disks are not cleaned up and libvirt restart is not handled. --- examples/object-events/event-test.c | 3 + include/libvirt/libvirt-domain-backup.h | 45 +++ include/libvirt/libvirt-domain.h | 3 + include/libvirt/libvirt.h | 1 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 2 + src/Makefile.am | 3 + src/access/viraccessperm.c | 3 +- src/access/viraccessperm.h | 6 + src/conf/backup_conf.c | 295 ++++++++++++++ src/conf/backup_conf.h | 85 ++++ src/conf/domain_conf.c | 2 +- src/driver-hypervisor.h | 11 + src/libvirt-domain-backup.c | 86 ++++ src/libvirt_private.syms | 6 + src/libvirt_public.syms | 2 + src/qemu/qemu_blockjob.c | 2 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_domain.h | 4 + src/qemu/qemu_driver.c | 684 ++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 32 ++ src/qemu/qemu_monitor.h | 12 + src/qemu/qemu_monitor_json.c | 105 +++++ src/qemu/qemu_monitor_json.h | 16 + src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 33 +- src/util/virerror.c | 1 + tools/Makefile.am | 1 + tools/virsh-backup.c | 150 +++++++ tools/virsh-backup.h | 28 ++ tools/virsh-domain.c | 3 +- tools/virsh.c | 2 + tools/virsh.h | 1 + 33 files changed, 1627 insertions(+), 4 deletions(-) create mode 100644 include/libvirt/libvirt-domain-backup.h create mode 100644 src/conf/backup_conf.c create mode 100644 src/conf/backup_conf.h create mode 100644 src/libvirt-domain-backup.c create mode 100644 tools/virsh-backup.c create mode 100644 tools/virsh-backup.h diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index 730cb8b..08490bb 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -829,6 +829,9 @@ blockJobTypeToStr(int type) case VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT: return "active layer block commit"; + + case VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP: + return "block backup"; } return "unknown"; diff --git a/include/libvirt/libvirt-domain-backup.h b/include/libvirt/libvirt-domain-backup.h new file mode 100644 index 0000000..cd24995 --- /dev/null +++ b/include/libvirt/libvirt-domain-backup.h @@ -0,0 +1,45 @@ +/* + * libvirt-domain-backup.h + * Summary: APIs for management of domain backups + * Description: Provides APIs for the management of domain backups + * Author: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; + * + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef __VIR_LIBVIRT_DOMAIN_BACKUP_H__ +# define __VIR_LIBVIRT_DOMAIN_BACKUP_H__ + +# ifndef __VIR_LIBVIRT_H_INCLUDES__ +# error "Don't include this file directly, only use libvirt/libvirt.h" +# endif + +typedef enum { + VIR_DOMAIN_BACKUP_START_QUIESCE = (1 << 0), /* use guest agent to + quiesce all mounted + file systems within + the domain */ +} virDomainBackupStartFlags; + +int virDomainBackupStart(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags); + +int virDomainBackupStop(virDomainPtr domaine, + unsigned int flags); + + +#endif /* __VIR_LIBVIRT_DOMAIN_BACKUP_H__ */ diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 6e0e7fb..f2cb759 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2048,6 +2048,9 @@ typedef enum { /* Active Block Commit (virDomainBlockCommit with flags), job * exists as long as sync is active */ + VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP = 5, + /* Block Backup */ + # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_BLOCK_JOB_TYPE_LAST # endif diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 36f6d60..be0d570 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -37,6 +37,7 @@ extern "C" { # include <libvirt/libvirt-host.h> # include <libvirt/libvirt-domain.h> # include <libvirt/libvirt-domain-snapshot.h> +# include <libvirt/libvirt-domain-backup.h> # include <libvirt/libvirt-event.h> # include <libvirt/libvirt-interface.h> # include <libvirt/libvirt-network.h> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 2ec560e..c1f8c6c 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -131,6 +131,7 @@ typedef enum { VIR_FROM_XENXL = 64, /* Error from Xen xl config code */ VIR_FROM_PERF = 65, /* Error from perf */ + VIR_FROM_DOMAIN_BACKUP = 66,/* Error from domain backup */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/po/POTFILES.in b/po/POTFILES.in index 25dbc84..4cdeb2f 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -18,6 +18,7 @@ src/bhyve/bhyve_driver.c src/bhyve/bhyve_monitor.c src/bhyve/bhyve_parse_command.c src/bhyve/bhyve_process.c +src/conf/backup_conf.c src/conf/capabilities.c src/conf/cpu_conf.c src/conf/device_conf.c @@ -281,6 +282,7 @@ src/xenconfig/xen_xl.c src/xenconfig/xen_xm.c tests/virpolkittest.c tools/libvirt-guests.sh.in +tools/virsh-backup.c tools/virsh-console.c tools/virsh-domain-monitor.c tools/virsh-domain.c diff --git a/src/Makefile.am b/src/Makefile.am index 8ee5567..c04e72c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -219,6 +219,7 @@ DRIVER_SOURCES = \ libvirt.c libvirt_internal.h \ libvirt-domain.c \ libvirt-domain-snapshot.c \ + libvirt-domain-backup.c \ libvirt-host.c \ libvirt-interface.c \ libvirt-network.c \ @@ -334,6 +335,7 @@ DOMAIN_CONF_SOURCES = \ conf/domain_audit.c conf/domain_audit.h \ conf/domain_nwfilter.c conf/domain_nwfilter.h \ conf/snapshot_conf.c conf/snapshot_conf.h \ + conf/backup_conf.c conf/backup_conf.h \ conf/numa_conf.c conf/numa_conf.h \ conf/virdomainobjlist.c conf/virdomainobjlist.h @@ -2390,6 +2392,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ libvirt.c \ libvirt-domain.c \ libvirt-domain-snapshot.c \ + libvirt-domain-backup.c \ libvirt-host.c \ libvirt-interface.c \ libvirt-network.c \ diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index 0f58290..16216c0 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -43,7 +43,8 @@ VIR_ENUM_IMPL(virAccessPermDomain, "fs_trim", "fs_freeze", "block_read", "block_write", "mem_read", "open_graphics", "open_device", "screenshot", - "open_namespace", "set_time", "set_password"); + "open_namespace", "set_time", "set_password", + "backup"); VIR_ENUM_IMPL(virAccessPermInterface, VIR_ACCESS_PERM_INTERFACE_LAST, diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 1817da7..06d5184 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -306,6 +306,12 @@ typedef enum { */ VIR_ACCESS_PERM_DOMAIN_SET_PASSWORD, + /** + * @desc: Backup domain + * @message: Backing domain up requires authorization + */ + VIR_ACCESS_PERM_DOMAIN_BACKUP, /* Backup domain */ + VIR_ACCESS_PERM_DOMAIN_LAST, } virAccessPermDomain; diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c new file mode 100644 index 0000000..ed00922 --- /dev/null +++ b/src/conf/backup_conf.c @@ -0,0 +1,295 @@ +/* + * backup_conf.c: domain backup XML processing + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; + */ + +#include <config.h> + +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/time.h> +#include <unistd.h> + +#include "internal.h" +#include "virbitmap.h" +#include "virbuffer.h" +#include "count-one-bits.h" +#include "datatypes.h" +#include "domain_conf.h" +#include "virlog.h" +#include "viralloc.h" +#include "netdev_bandwidth_conf.h" +#include "netdev_vport_profile_conf.h" +#include "nwfilter_conf.h" +#include "secret_conf.h" +#include "backup_conf.h" +#include "virstoragefile.h" +#include "viruuid.h" +#include "virfile.h" +#include "virerror.h" +#include "virxml.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_DOMAIN_BACKUP + +VIR_LOG_INIT("conf.backup_conf"); + +VIR_ENUM_IMPL(virDomainBackupAddress, VIR_DOMAIN_BACKUP_ADDRESS_LAST, + "ip", + "unix") + +static int +virDomainBackupDiskDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virDomainBackupDiskDefPtr def) +{ + int ret = -1; + char *present = NULL; + char *type = NULL; + xmlNodePtr cur; + xmlNodePtr saved = ctxt->node; + + ctxt->node = node; + + if ((type = virXMLPropString(node, "type")) && + virStorageTypeFromString(type) != VIR_STORAGE_TYPE_FILE) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'file' is the only supported backup source type")); + goto cleanup; + } + + if ((cur = virXPathNode("./source", ctxt))) { + if (VIR_ALLOC(def->src) < 0) + goto cleanup; + + def->src->type = VIR_STORAGE_TYPE_FILE; + def->src->format = VIR_STORAGE_FILE_QCOW2; + + if (virDomainDiskSourceParse(cur, ctxt, def->src) < 0) + goto cleanup; + } + + def->name = virXMLPropString(node, "name"); + if (!def->name) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing name from disk backup element")); + goto cleanup; + } + + present = virXMLPropString(node, "present"); + if (present && (def->present = virTristateBoolTypeFromString(present)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid disk '%s' present attribute value"), + def->name); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(present); + VIR_FREE(type); + + ctxt->node = saved; + + return ret; +} + +static int +virDomainBackupAddressDefParseXML(xmlNodePtr node, + virDomainBackupAddressDefPtr def) +{ + char *type = virXMLPropString(node, "type"); + char *port = NULL; + int ret = -1; + + if (!type) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("backup address type must be specified")); + goto cleanup; + } + + if ((def->type = virDomainBackupAddressTypeFromString(type)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown backup address type '%s'"), type); + goto cleanup; + } + + switch (def->type) { + case VIR_DOMAIN_BACKUP_ADDRESS_IP: + def->data.ip.host = virXMLPropString(node, "host"); + port = virXMLPropString(node, "port"); + if (!def->data.ip.host || !port) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("both host and port must be specified " + "for ip address type")); + goto cleanup; + } + if (virStrToLong_i(port, NULL, 10, &def->data.ip.port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse port %s"), port); + goto cleanup; + } + break; + case VIR_DOMAIN_BACKUP_ADDRESS_UNIX: + def->data.socket.path = virXMLPropString(node, "path"); + if (!def->data.socket.path) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("path must be specified for unix address type")); + goto cleanup; + } + break; + } + + ret = 0; + + cleanup: + VIR_FREE(type); + VIR_FREE(port); + + return ret; +} + +static virDomainBackupDefPtr +virDomainBackupDefParse(xmlXPathContextPtr ctxt) +{ + virDomainBackupDefPtr def = NULL; + virDomainBackupDefPtr ret = NULL; + xmlNodePtr *nodes = NULL, node; + size_t i; + int n; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (!(node = virXPathNode("./address[1]", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("export address must be specifed")); + goto cleanup; + } + + if (virDomainBackupAddressDefParseXML(node, &def->address) < 0) + goto cleanup; + + if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0) + goto cleanup; + + if (n && VIR_ALLOC_N(def->disks, n) < 0) + goto cleanup; + def->ndisks = n; + for (i = 0; i < def->ndisks; i++) { + if (virDomainBackupDiskDefParseXML(nodes[i], ctxt, &def->disks[i]) < 0) + goto cleanup; + } + + ret = def; + def = NULL; + + cleanup: + VIR_FREE(nodes); + virDomainBackupDefFree(def); + + return ret; +} + +virDomainBackupDefPtr +virDomainBackupDefParseNode(xmlDocPtr xml, + xmlNodePtr root, + virCapsPtr caps ATTRIBUTE_UNUSED, + virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED, + unsigned int flags) +{ + xmlXPathContextPtr ctxt = NULL; + virDomainBackupDefPtr def = NULL; + + virCheckFlags(0, NULL); + + if (!xmlStrEqual(root->name, BAD_CAST "domainbackup")) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("domainbackup")); + goto cleanup; + } + + ctxt = xmlXPathNewContext(xml); + if (ctxt == NULL) { + virReportOOMError(); + goto cleanup; + } + + ctxt->node = root; + def = virDomainBackupDefParse(ctxt); + + cleanup: + xmlXPathFreeContext(ctxt); + + return def; +} + + +virDomainBackupDefPtr +virDomainBackupDefParseString(const char *xmlStr, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + virDomainBackupDefPtr ret = NULL; + xmlDocPtr xml; + int keepBlanksDefault = xmlKeepBlanksDefault(0); + + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)")))) { + xmlKeepBlanksDefault(keepBlanksDefault); + ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml), + caps, xmlopt, flags); + xmlFreeDoc(xml); + } + xmlKeepBlanksDefault(keepBlanksDefault); + + return ret; +} + +static +void virDomainBackupAddressDefFree(virDomainBackupAddressDefPtr def) +{ + switch (def->type) { + case VIR_DOMAIN_BACKUP_ADDRESS_IP: + VIR_FREE(def->data.ip.host); + break; + case VIR_DOMAIN_BACKUP_ADDRESS_UNIX: + VIR_FREE(def->data.socket.path); + break; + } +} + +void virDomainBackupDefFree(virDomainBackupDefPtr def) +{ + size_t i; + + if (!def) + return; + + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDefPtr disk = &def->disks[i]; + + virStorageSourceFree(disk->src); + VIR_FREE(disk->name); + } + VIR_FREE(def->disks); + + virDomainBackupAddressDefFree(&def->address); + + VIR_FREE(def); +} diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h new file mode 100644 index 0000000..1b09647 --- /dev/null +++ b/src/conf/backup_conf.h @@ -0,0 +1,85 @@ +/* + * backup_conf.h: domain backup XML processing + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; + */ + +#ifndef __BACKUP_CONF_H +# define __BACKUP_CONF_H + +# include "internal.h" +# include "domain_conf.h" + +typedef struct _virDomainBackupDiskDef virDomainBackupDiskDef; +typedef virDomainBackupDiskDef *virDomainBackupDiskDefPtr; +struct _virDomainBackupDiskDef { + char *name; /* name matching the <target dev='...' of the domain */ + int present; /* enum virTristateBool */ + int idx; /* index within dom->disks that matches name */ + virStorageSourcePtr src; +}; + +typedef enum { + VIR_DOMAIN_BACKUP_ADDRESS_IP, + VIR_DOMAIN_BACKUP_ADDRESS_UNIX, + + VIR_DOMAIN_BACKUP_ADDRESS_LAST, +} virDomainBackupAddressType; + +VIR_ENUM_DECL(virDomainBackupAddress) + +typedef struct _virDomainBackupAddressDef virDomainBackupAddressDef; +typedef virDomainBackupAddressDef *virDomainBackupAddressDefPtr; +struct _virDomainBackupAddressDef { + union { + struct { + char *host; + int port; + } ip; + struct { + char *path; + } socket; + } data; + int type; /* virDomainBackupAddress */ +}; + +/* Stores the complete backup metadata */ +typedef struct _virDomainBackupDef virDomainBackupDef; +typedef virDomainBackupDef *virDomainBackupDefPtr; +struct _virDomainBackupDef { + virDomainBackupAddressDef address; + + size_t ndisks; + virDomainBackupDiskDef *disks; + + virDomainDefPtr dom; +}; + +virDomainBackupDefPtr virDomainBackupDefParseString(const char *xmlStr, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); + +virDomainBackupDefPtr virDomainBackupDefParseNode(xmlDocPtr xml, + xmlNodePtr root, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); + +void virDomainBackupDefFree(virDomainBackupDefPtr def); + +#endif /* __BACKUP_CONF_H */ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c8c4f61..f0247e2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -838,7 +838,7 @@ VIR_ENUM_IMPL(virDomainLoader, * <mirror> XML (remaining types are not two-phase). */ VIR_ENUM_DECL(virDomainBlockJob) VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST, - "", "", "copy", "", "active-commit") + "", "", "copy", "", "active-commit", "") VIR_ENUM_IMPL(virDomainMemoryModel, VIR_DOMAIN_MEMORY_MODEL_LAST, "", "dimm") diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 5cd1fdf..63ada62 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1251,6 +1251,15 @@ typedef int int state, unsigned int flags); +typedef int +(*virDrvDomainBackupStart)(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags); + +typedef int +(*virDrvDomainBackupStop)(virDomainPtr domain, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1489,6 +1498,8 @@ struct _virHypervisorDriver { virDrvDomainMigrateStartPostCopy domainMigrateStartPostCopy; virDrvDomainGetGuestVcpus domainGetGuestVcpus; virDrvDomainSetGuestVcpus domainSetGuestVcpus; + virDrvDomainBackupStart domainBackupStart; + virDrvDomainBackupStop domainBackupStop; }; diff --git a/src/libvirt-domain-backup.c b/src/libvirt-domain-backup.c new file mode 100644 index 0000000..e4b8a7b --- /dev/null +++ b/src/libvirt-domain-backup.c @@ -0,0 +1,86 @@ +/* + * libvirt-domain-backup.c: entry points for virDomainBackupPtr APIs + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; + */ + +#include <config.h> + +#include "datatypes.h" +#include "virlog.h" + +VIR_LOG_INIT("libvirt.domain-backup"); + +#define VIR_FROM_THIS VIR_FROM_DOMAIN_BACKUP + +int +virDomainBackupStart(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "xmlDesc=%s, flags=%x", xmlDesc, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckNonNullArgGoto(xmlDesc, error); + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainBackupStart) { + if (conn->driver->domainBackupStart(domain, xmlDesc, flags) < 0) + goto error; + + return 0; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} + +int +virDomainBackupStop(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainBackupStop) { + if (conn->driver->domainBackupStop(domain, flags) < 0) + goto error; + + return 0; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d62c74c..e0ae661 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -42,6 +42,12 @@ virAccessPermStorageVolTypeFromString; virAccessPermStorageVolTypeToString; +# conf/backup_conf.h +virDomainBackupDefFree; +virDomainBackupDefParseNode; +virDomainBackupDefParseString; + + # conf/capabilities.h virCapabilitiesAddGuest; virCapabilitiesAddGuestDomain; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index e01604c..8ea164e 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -744,6 +744,8 @@ LIBVIRT_2.2.0 { global: virConnectNodeDeviceEventRegisterAny; virConnectNodeDeviceEventDeregisterAny; + virDomainBackupStart; + virDomainBackupStop; } LIBVIRT_2.0.0; # .... define new API here using predicted next version number .... diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 83a5a3f..66e4ea7 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -171,6 +171,8 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, break; case VIR_DOMAIN_BLOCK_JOB_FAILED: + if (diskPriv->backuping) + diskPriv->backupFailed = true; case VIR_DOMAIN_BLOCK_JOB_CANCELED: virStorageSourceFree(disk->mirror); disk->mirror = NULL; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 510cd9a..43f85bf 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -32,6 +32,7 @@ # include "network_conf.h" # include "domain_conf.h" # include "snapshot_conf.h" +# include "backup_conf.h" # include "domain_event.h" # include "virthread.h" # include "security/security_manager.h" diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ea57111..527ecb0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -284,6 +284,10 @@ struct _qemuDomainDiskPrivate { * single disk */ bool blockjob; + bool backuping; + bool backupdev; + bool backupFailed; + /* for some synchronous block jobs, we need to notify the owner */ int blockJobType; /* type of the block job from the event */ int blockJobStatus; /* status of the finished block job */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 807e06d..9b0cb12 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20118,6 +20118,688 @@ qemuDomainSetGuestVcpus(virDomainPtr dom, return ret; } +/** + * FIXME nearly copy-paste of virDomainSnapshotDefAssignExternalNames + * + */ +static int +virDomainBackupDefGeneratePaths(virDomainBackupDefPtr def) +{ + char *tmppath; + char *tmp; + size_t i; + size_t j; + + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDefPtr disk = &def->disks[i]; + + if (disk->present != VIR_TRISTATE_BOOL_YES || disk->src) + continue; + + if (VIR_ALLOC(disk->src) < 0) + return -1; + + disk->src->type = VIR_STORAGE_TYPE_FILE; + disk->src->format = VIR_STORAGE_FILE_QCOW2; + + if (VIR_STRDUP(tmppath, virDomainDiskGetSource(def->dom->disks[i])) < 0) + return -1; + + /* drop suffix of the file name */ + if ((tmp = strrchr(tmppath, '.')) && !strchr(tmp, '/')) + *tmp = '\0'; + + if (virAsprintf(&disk->src->path, "%s.backup", tmppath) < 0) { + VIR_FREE(tmppath); + return -1; + } + + VIR_FREE(tmppath); + + /* verify that we didn't generate a duplicate name */ + for (j = 0; j < i; j++) { + if (STREQ_NULLABLE(disk->src->path, def->disks[j].src->path)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot generate backup name for " + "disk '%s': collision with disk '%s'"), + disk->name, def->disks[j].name); + return -1; + } + } + } + + return 0; +} + +static int +virDomainBackupCompareDiskIndex(const void *a, const void *b) +{ + const virDomainBackupDiskDef *diska = a; + const virDomainBackupDiskDef *diskb = b; + + /* Integer overflow shouldn't be a problem here. */ + return diska->idx - diskb->idx; +} + +static int +virDomainBackupAlignDisks(virDomainBackupDefPtr def) +{ + int ret = -1; + virBitmapPtr map = NULL; + size_t i; + int ndisks; + + if (!def->dom) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing domain in backup")); + goto cleanup; + } + + if (!(map = virBitmapNew(def->dom->ndisks))) + goto cleanup; + + /* Double check requested disks. */ + for (i = 0; i < def->ndisks; i++) { + virDomainBackupDiskDefPtr disk = &def->disks[i]; + int idx = virDomainDiskIndexByName(def->dom, disk->name, false); + + if (idx < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("no disk named '%s'"), disk->name); + goto cleanup; + } + + if (virBitmapIsBitSet(map, idx)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' specified twice"), disk->name); + goto cleanup; + } + ignore_value(virBitmapSetBit(map, idx)); + disk->idx = idx; + + if (STRNEQ(disk->name, def->dom->disks[idx]->dst)) { + VIR_FREE(disk->name); + if (VIR_STRDUP(disk->name, def->dom->disks[idx]->dst) < 0) + goto cleanup; + } + + if (!disk->present) + disk->present = VIR_TRISTATE_BOOL_YES; + + if (virStorageSourceIsEmpty(def->dom->disks[i]->src) && + disk->present == VIR_TRISTATE_BOOL_YES) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' has no source, can not be exported"), + disk->name); + } + } + + /* Provide defaults for all remaining disks. */ + ndisks = def->ndisks; + if (VIR_EXPAND_N(def->disks, def->ndisks, + def->dom->ndisks - def->ndisks) < 0) + goto cleanup; + + for (i = 0; i < def->dom->ndisks; i++) { + virDomainBackupDiskDefPtr disk; + + if (virBitmapIsBitSet(map, i)) + continue; + + disk = &def->disks[ndisks++]; + if (VIR_STRDUP(disk->name, def->dom->disks[i]->dst) < 0) + goto cleanup; + disk->idx = i; + + if (virStorageSourceIsEmpty(def->dom->disks[i]->src) || + def->dom->disks[i]->src->readonly) + disk->present = VIR_TRISTATE_BOOL_NO; + else + disk->present = VIR_TRISTATE_BOOL_YES; + } + + qsort(&def->disks[0], def->ndisks, sizeof(def->disks[0]), + virDomainBackupCompareDiskIndex); + + if (virDomainBackupDefGeneratePaths(def) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virBitmapFree(map); + return ret; +} + + +/* + * FIXME has much in common with qemuMigrationCancelOneDriveMirror + */ +static int +qemuBackupDriveCancelSingle(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char *diskAlias = NULL; + int ret = -1; + int status; + int rv; + + status = qemuBlockJobUpdate(driver, vm, disk); + if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("disk %s backup failed"), disk->dst); + goto cleanup; + } + + if (!(diskAlias = qemuAliasFromDisk(disk))) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + rv = qemuMonitorBlockJobCancel(priv->mon, diskAlias, true); + /* -2 means race condition, job is already failed but + * event is not yet delivered, thus we continue as + * in case of success, next block job update will deliver the event */ + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv == -1) + goto cleanup; + + ret = 0; + + cleanup: + if (ret < 0) { + qemuBlockJobSyncEnd(driver, vm, disk); + QEMU_DOMAIN_DISK_PRIVATE(disk)->backuping = false; + QEMU_DOMAIN_DISK_PRIVATE(disk)->backupFailed = false; + } + + VIR_FREE(diskAlias); + + return ret; +} + +static bool +qemuBackupDriveCancelled(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virErrorPtr *err) +{ + size_t i; + size_t active = 0; + int status; + + *err = NULL; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (!diskPriv->backuping) + continue; + + status = qemuBlockJobUpdate(driver, vm, disk); + switch (status) { + case VIR_DOMAIN_BLOCK_JOB_FAILED: + virReportError(VIR_ERR_OPERATION_FAILED, + _("migration of disk %s failed"), disk->dst); + if (!*err) + *err = virSaveLastError(); + /* fallthrough */ + case VIR_DOMAIN_BLOCK_JOB_CANCELED: + qemuBlockJobSyncEnd(driver, vm, disk); + diskPriv->backuping = false; + diskPriv->backupFailed = false; + break; + + default: + ++active; + } + } + + return active == 0; +} + +/** + * FIXME nearly copy paste from qemuMigrationCancelDriveMirror + * + * Cancel backup jobs for all disks + * + * Returns 0 on success, -1 otherwise. + */ +static int +qemuBackupDriveCancelAll(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virErrorPtr err = NULL, wait_err; + size_t i; + int ret = -1; + + VIR_DEBUG("Cancelling drive mirrors for domain %s", vm->def->name); + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (!diskPriv->backuping) + continue; + + if (qemuBackupDriveCancelSingle(driver, vm, disk) < 0) { + if (!virDomainObjIsActive(vm)) + goto cleanup; + + if (!err) + err = virSaveLastError(); + } + } + + while (!qemuBackupDriveCancelled(driver, vm, &wait_err)) { + if (!err && wait_err) + err = wait_err; + else + virFreeError(wait_err); + + if (virDomainObjWait(vm) < 0) + goto cleanup; + } + + if (!err) + ret = 0; + + cleanup: + if (err) { + virSetError(err); + virFreeError(err); + } + + return ret; +} + +static int +qemuDomainBackupFinishExport(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr err = NULL; + char *dev = NULL; + int ret = -1; + size_t i; + + qemuDomainObjEnterMonitor(driver, vm); + ignore_value(qemuMonitorNBDServerStop(priv->mon)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + if (qemuBackupDriveCancelAll(driver, vm) < 0) { + if (!virDomainObjIsActive(vm)) + goto cleanup; + + err = virSaveLastError(); + } + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (!diskPriv->backupdev) + continue; + diskPriv->backupdev = false; + + if (virAsprintf(&dev, "backup-%s", disk->info.alias) < 0) + continue; + + qemuDomainObjEnterMonitor(driver, vm); + ignore_value(qemuMonitorBlockdevDel(priv->mon, dev)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + VIR_FREE(dev); + } + + if (!err) + ret = 0; + cleanup: + if (err) { + virSetError(err); + virFreeError(err); + } + VIR_FREE(dev); + + return ret; +} + +static int +qemuDomainBackupPrepareDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virStorageSourcePtr src, + struct qemuDomainDiskInfo *info) +{ + char *path = NULL; + virCommandPtr cmd = NULL; + int ret = -1; + const char *qemuImgPath; + + if (!(qemuImgPath = qemuFindQemuImgBinary(driver))) + goto cleanup; + + if (qemuGetDriveSourceString(src, NULL, &path) < 0) + goto cleanup; + + if (qemuDomainStorageFileInit(driver, vm, src) < 0) + goto cleanup; + + if (virStorageFileCreate(src) < 0) { + virReportSystemError(errno, _("failed to create image file '%s'"), + path); + goto cleanup; + } + + if (!(cmd = virCommandNewArgList(qemuImgPath, "create", + "-f", "qcow2", + NULL))) + goto cleanup; + + virCommandAddArg(cmd, src->path); + virCommandAddArgFormat(cmd, "%lli", info->guest_size); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virStorageFileDeinit(src); + VIR_FREE(path); + virCommandFree(cmd); + + return ret; +} + +static int +qemuDomainBackupExportDisks(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainBackupDefPtr def) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char *dev = NULL; + char *dev_backup = NULL; + int ret = -1, rc; + virJSONValuePtr actions = NULL; + size_t i; + virHashTablePtr block_info = NULL; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorNBDServerStart(priv->mon, def->address.data.ip.host, + def->address.data.ip.port); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; + + if (!(actions = virJSONValueNewArray())) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + block_info = qemuMonitorGetBlockInfo(priv->mon); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !block_info) + goto cleanup; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + struct qemuDomainDiskInfo *disk_info; + + if (def->disks[i].present != VIR_TRISTATE_BOOL_YES) + continue; + + if (!(disk_info = virHashLookup(block_info, disk->info.alias))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing block info for '%s'"), disk->info.alias); + goto cleanup; + } + + if (qemuDomainBackupPrepareDisk(driver, vm, def->disks[i].src, + disk_info) < 0) + goto cleanup; + + if (virAsprintf(&dev, "drive-%s", disk->info.alias) < 0) + goto cleanup; + if (virAsprintf(&dev_backup, "backup-%s", disk->info.alias) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorBlockdevAdd(priv->mon, dev_backup, + def->disks[i].src->path); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + QEMU_DOMAIN_DISK_PRIVATE(disk)->backupdev = true; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorBlockdevBackup(actions, dev, dev_backup, 0); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + VIR_FREE(dev); + VIR_FREE(dev_backup); + } + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorTransaction(priv->mon, actions); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + if (def->disks[i].present != VIR_TRISTATE_BOOL_YES) + continue; + + QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = true; + QEMU_DOMAIN_DISK_PRIVATE(disk)->backuping = true; + qemuBlockJobSyncBegin(disk); + } + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + if (def->disks[i].present != VIR_TRISTATE_BOOL_YES) + continue; + + if (virAsprintf(&dev, "drive-%s", disk->info.alias) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorNBDServerAdd(priv->mon, dev, false); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + virJSONValueFree(actions); + VIR_FREE(dev); + VIR_FREE(dev_backup); + virHashFree(block_info); + + return ret; +} + +static int +qemuDomainBackupStart(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags) +{ + virQEMUDriverPtr driver = domain->conn->privateData; + virCapsPtr caps = NULL; + virDomainBackupDefPtr def = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + bool job = false; + int freezed = -1; + size_t i; + + virCheckFlags(VIR_DOMAIN_BACKUP_START_QUIESCE, + -1); + + if (!(vm = qemuDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainBackupStartEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (!(def = virDomainBackupDefParseString(xmlDesc, caps, driver->xmlopt, 0))) + goto cleanup; + + /* FIXME refactor start nbd monitor function to support unix sockets */ + if (def->address.type != VIR_DOMAIN_BACKUP_ADDRESS_IP) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("backup thru unix sockets is not supported")); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("backing up inactive domains is not supported")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + job = true; + + def->dom = vm->def; + + if (virDomainBackupAlignDisks(def) < 0) + goto cleanup; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (def->disks[i].present != VIR_TRISTATE_BOOL_YES) + continue; + + if (diskPriv->blockjob) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' already has a blockjob"), disk->dst); + goto cleanup; + } + } + + /* FIXME remove snapshot from below function name */ + if ((flags & VIR_DOMAIN_BACKUP_START_QUIESCE) && + (freezed = qemuDomainSnapshotFSFreeze(driver, vm, NULL, 0)) < 0) + goto cleanup; + + if (qemuDomainBackupExportDisks(driver, vm, def) < 0) + goto cleanup; + + ret = 0; + + cleanup: + /* FIXME remove snapshot from name below */ + if (freezed != -1 && qemuDomainSnapshotFSThaw(driver, vm, ret == 0) < 0) + ret = -1; + + if (ret < 0 && virDomainObjIsActive(vm)) { + virErrorPtr err = virSaveLastError(); + + ignore_value(qemuDomainBackupFinishExport(driver, vm)); + virSetError(err); + virFreeError(err); + } else if (ret == 0) { + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (!diskPriv->backuping) + continue; + + qemuBlockJobSyncEnd(driver, vm, disk); + } + } + + if (job) + qemuDomainObjEndJob(driver, vm); + + virDomainBackupDefFree(def); + virObjectUnref(caps); + virDomainObjEndAPI(&vm); + + return ret; +} + +static int +qemuDomainBackupStop(virDomainPtr domain, + unsigned int flags) +{ + virQEMUDriverPtr driver = domain->conn->privateData; + virDomainObjPtr vm = NULL; + virErrorPtr err = NULL; + int ret = -1; + bool job = false; + size_t i; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainBackupStopEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("inactive domain can't have active backup")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + job = true; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (!diskPriv->backuping) + continue; + + /* backup blockjob can fail/cancelled between start and stop */ + if (!diskPriv->blockjob) { + diskPriv->backuping = false; + + if (diskPriv->backupFailed) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("disk %s backup failed"), disk->dst); + if (!err) + err = virSaveLastError(); + diskPriv->backupFailed = false; + } + continue; + } + + qemuBlockJobSyncBegin(disk); + } + + if (qemuDomainBackupFinishExport(driver, vm) < 0) + goto cleanup; + + if (!err) + ret = 0; + + cleanup: + if (job) + qemuDomainObjEndJob(driver, vm); + + virDomainObjEndAPI(&vm); + if (err) { + virSetError(err); + virFreeError(err); + } + + return ret; +} static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, @@ -20332,6 +21014,8 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainMigrateStartPostCopy = qemuDomainMigrateStartPostCopy, /* 1.3.3 */ .domainGetGuestVcpus = qemuDomainGetGuestVcpus, /* 2.0.0 */ .domainSetGuestVcpus = qemuDomainSetGuestVcpus, /* 2.0.0 */ + .domainBackupStart = qemuDomainBackupStart, /* 2.3.0 */ + .domainBackupStop = qemuDomainBackupStop, /* 2.3.0 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 944856d..dad61bd 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4001,3 +4001,35 @@ qemuMonitorGetRTCTime(qemuMonitorPtr mon, return qemuMonitorJSONGetRTCTime(mon, tm); } + +int qemuMonitorBlockdevBackup(virJSONValuePtr actions, + const char *device, + const char *target, + unsigned long long speed) +{ + VIR_DEBUG("actions=%p, device=%s, target=%s, speed=%llu", + actions, device, target, speed); + + return qemuMonitorJSONBlockdevBackup(actions, device, target, speed); +} + +int qemuMonitorBlockdevAdd(qemuMonitorPtr mon, + const char *id, + const char *path) +{ + VIR_DEBUG("mon=%p, id=%s, path=%s", mon, id, path); + + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONBlockdevAdd(mon, id, path); +} + +int qemuMonitorBlockdevDel(qemuMonitorPtr mon, + const char *id) +{ + VIR_DEBUG("mon=%p, id=%s", mon, id); + + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONBlockdevDel(mon, id); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b838725..3cfd32b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -991,4 +991,16 @@ int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon); int qemuMonitorGetRTCTime(qemuMonitorPtr mon, struct tm *tm); +int qemuMonitorBlockdevAdd(qemuMonitorPtr mon, + const char *id, + const char *path); + +int qemuMonitorBlockdevDel(qemuMonitorPtr mon, + const char *id); + +int qemuMonitorBlockdevBackup(virJSONValuePtr actions, + const char *device, + const char *target, + unsigned long long speed); + #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 42b05c4..9ee025a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -831,6 +831,8 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon, type = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT; else if (STREQ(type_str, "mirror")) type = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; + else if (STREQ(type_str, "backup")) + type = VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP; switch ((virConnectDomainEventBlockJobStatus) event) { case VIR_DOMAIN_BLOCK_JOB_COMPLETED: @@ -7260,3 +7262,106 @@ qemuMonitorJSONGetHotpluggableCPUs(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + +int qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon, + const char *id, + const char *path) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr args = NULL; + virJSONValuePtr file = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr options = NULL; + + if (!(cmd = virJSONValueNewObject()) || + !(args = virJSONValueNewObject()) || + !(options = virJSONValueNewObject()) || + !(file = virJSONValueNewObject())) + goto cleanup; + + if (virJSONValueObjectAppendString(file, "driver", "file") < 0 || + virJSONValueObjectAppendString(file, "filename", path) < 0) + goto cleanup; + + if (virJSONValueObjectAppendString(options, "driver", "qcow2") < 0 || + virJSONValueObjectAppendString(options, "id", id) < 0 || + virJSONValueObjectAppend(options, "file", file) < 0) + goto cleanup; + + if (virJSONValueObjectAppend(args, "options", options) < 0) + goto cleanup; + + if (virJSONValueObjectAppendString(cmd, "execute", "blockdev-add") < 0 || + virJSONValueObjectAppend(cmd, "arguments", args) < 0) + goto cleanup; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(args); + virJSONValueFree(file); + virJSONValueFree(reply); + virJSONValueFree(options); + return ret; +} + +int qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon, + const char *id) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("x-blockdev-del", + "s:id", id, + NULL); + if (!cmd) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +int qemuMonitorJSONBlockdevBackup(virJSONValuePtr actions, + const char *device, + const char *target, + unsigned long long speed) +{ + int ret = -1; + virJSONValuePtr cmd; + + cmd = qemuMonitorJSONMakeCommandRaw(true, + "blockdev-backup", + "s:device", device, + "s:target", target, + "s:sync", "none", + "Y:speed", speed, + NULL); + if (!cmd) + return -1; + + if (virJSONValueArrayAppend(actions, cmd) < 0) + goto cleanup; + + ret = 0; + cmd = NULL; + cleanup: + virJSONValueFree(cmd); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 0eab96f..8ae6c1d 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -499,4 +499,20 @@ int qemuMonitorJSONGetHotpluggableCPUs(qemuMonitorPtr mon, struct qemuMonitorQueryHotpluggableCpusEntry **entries, size_t *nentries) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +int qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon, + const char *id, + const char *path) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +int qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon, + const char *id) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int qemuMonitorJSONBlockdevBackup(virJSONValuePtr actions, + const char *device, + const char *target, + unsigned long long speed) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 3b8b796..52e53f5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8164,6 +8164,8 @@ static virHypervisorDriver hypervisor_driver = { .domainMigrateStartPostCopy = remoteDomainMigrateStartPostCopy, /* 1.3.3 */ .domainGetGuestVcpus = remoteDomainGetGuestVcpus, /* 2.0.0 */ .domainSetGuestVcpus = remoteDomainSetGuestVcpus, /* 2.0.0 */ + .domainBackupStart = remoteDomainBackupStart, /* 2.3.0 */ + .domainBackupStop = remoteDomainBackupStop, /* 2.3.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 8a8e263..c62ee0e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2679,6 +2679,25 @@ struct remote_domain_snapshot_delete_args { unsigned int flags; }; +struct remote_domain_backup_start_args { + remote_nonnull_domain dom; + remote_nonnull_string xml_desc; + unsigned int flags; +}; + +struct remote_domain_backup_start_ret { + int result; +}; + +struct remote_domain_backup_stop_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_backup_stop_ret { + int result; +}; + struct remote_domain_open_console_args { remote_nonnull_domain dom; remote_string dev_name; @@ -5934,5 +5953,17 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_NODE_DEVICE_EVENT_UPDATE = 377 + REMOTE_PROC_NODE_DEVICE_EVENT_UPDATE = 377, + + /** + * @generate: both + * @acl: domain:backup + */ + REMOTE_PROC_DOMAIN_BACKUP_START = 378, + + /** + * @generate: both + * @acl: domain:backup + */ + REMOTE_PROC_DOMAIN_BACKUP_STOP = 379 }; diff --git a/src/util/virerror.c b/src/util/virerror.c index 1177570..62b5c62 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -138,6 +138,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Xen XL Config", "Perf", + "Domain Backup", ) diff --git a/tools/Makefile.am b/tools/Makefile.am index e7e42c3..1527c5f 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -201,6 +201,7 @@ virsh_SOURCES = \ virsh-secret.c virsh-secret.h \ virsh-snapshot.c virsh-snapshot.h \ virsh-volume.c virsh-volume.h \ + virsh-backup.c virsh-backup.h \ $(NULL) virsh_LDFLAGS = \ diff --git a/tools/virsh-backup.c b/tools/virsh-backup.c new file mode 100644 index 0000000..c5ed972 --- /dev/null +++ b/tools/virsh-backup.c @@ -0,0 +1,150 @@ +/* + * virsh-backup.c: Commands to manage domain backup + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; + */ + +#include <config.h> +#include "virsh-backup.h" + +#include "internal.h" +#include "virfile.h" +#include "virsh-domain.h" +#include "viralloc.h" + +#define VIRSH_COMMON_OPT_DOMAIN_FULL \ + VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid")) \ + +/* + * "backup-start" command + */ +static const vshCmdInfo info_backup_start[] = { + {.name = "help", + .data = N_("Start pull backup") + }, + {.name = "desc", + .data = N_("Start pull backup") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_backup_start[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL, + {.name = "xmlfile", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain backup XML") + }, + {.name = "quiesce", + .type = VSH_OT_BOOL, + .help = N_("quiesce guest's file systems") + }, + {.name = NULL} +}; + +static bool +cmdBackupStart(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + const char *from = NULL; + char *buffer = NULL; + unsigned int flags = 0; + + if (vshCommandOptBool(cmd, "quiesce")) + flags |= VIR_DOMAIN_BACKUP_START_QUIESCE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + goto cleanup; + + if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) + goto cleanup; + + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { + vshSaveLibvirtError(); + goto cleanup; + } + + if (virDomainBackupStart(dom, buffer, flags) < 0) + goto cleanup; + + vshPrint(ctl, _("Domain backup started")); + ret = true; + + cleanup: + VIR_FREE(buffer); + if (dom) + virDomainFree(dom); + + return ret; +} + +/* + * "backup-stop" command + */ +static const vshCmdInfo info_backup_stop[] = { + {.name = "help", + .data = N_("Stop pull backup") + }, + {.name = "desc", + .data = N_("Stop pull backup") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_backup_stop[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL, + {.name = NULL} +}; + +static bool +cmdBackupStop(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + goto cleanup; + + if (virDomainBackupStop(dom, 0) < 0) + goto cleanup; + + vshPrint(ctl, _("Domain backup stopped")); + ret = true; + + cleanup: + if (dom) + virDomainFree(dom); + + return ret; +} + +const vshCmdDef backupCmds[] = { + {.name = "backup-start", + .handler = cmdBackupStart, + .opts = opts_backup_start, + .info = info_backup_start, + .flags = 0 + }, + {.name = "backup-stop", + .handler = cmdBackupStop, + .opts = opts_backup_stop, + .info = info_backup_stop, + .flags = 0 + }, + {.name = NULL} +}; diff --git a/tools/virsh-backup.h b/tools/virsh-backup.h new file mode 100644 index 0000000..b765ad7 --- /dev/null +++ b/tools/virsh-backup.h @@ -0,0 +1,28 @@ +/* + * virsh-backup.h: Commands to manage domain backup + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; + */ + +#ifndef VIRSH_BACKUP_H +# define VIRSH_BACKUP_H + +# include "virsh.h" + +extern const vshCmdDef backupCmds[]; + +#endif /* VIRSH_BACKUP_H */ diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a614512..16a7b4c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2528,7 +2528,8 @@ VIR_ENUM_IMPL(virshDomainBlockJob, N_("Block Pull"), N_("Block Copy"), N_("Block Commit"), - N_("Active Block Commit")) + N_("Active Block Commit"), + N_("Block Backup")) static const char * virshDomainBlockJobToString(int type) diff --git a/tools/virsh.c b/tools/virsh.c index cb60edc..96e2862 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -71,6 +71,7 @@ #include "virsh-secret.h" #include "virsh-snapshot.h" #include "virsh-volume.h" +#include "virsh-backup.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -919,6 +920,7 @@ static const vshCmdGrp cmdGroups[] = { {VIRSH_CMD_GRP_NODEDEV, "nodedev", nodedevCmds}, {VIRSH_CMD_GRP_SECRET, "secret", secretCmds}, {VIRSH_CMD_GRP_SNAPSHOT, "snapshot", snapshotCmds}, + {VIRSH_CMD_GRP_BACKUP, "backup", backupCmds}, {VIRSH_CMD_GRP_STORAGE_POOL, "pool", storagePoolCmds}, {VIRSH_CMD_GRP_STORAGE_VOL, "volume", storageVolCmds}, {VIRSH_CMD_GRP_VIRSH, "virsh", virshCmds}, diff --git a/tools/virsh.h b/tools/virsh.h index fd552bb..839f36f 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -57,6 +57,7 @@ # define VIRSH_CMD_GRP_NWFILTER "Network Filter" # define VIRSH_CMD_GRP_SECRET "Secret" # define VIRSH_CMD_GRP_SNAPSHOT "Snapshot" +# define VIRSH_CMD_GRP_BACKUP "Backup" # define VIRSH_CMD_GRP_HOST_AND_HV "Host and Hypervisor" # define VIRSH_CMD_GRP_VIRSH "Virsh itself" -- 1.8.3.1

On 27.09.2016 01:04, John Ferlan wrote: Ok, at least the patch can be some kind of candidate to be pushed upstream as soon as blockdev-del drops x- prefix. Totally agree. Well it is just a flag, just as in case of snapshots. As to coding conventions etc a lot of stuff is rooted in similar snapshot code and xml. but there is VIR_FROM_DOMAIN_SNAPSHOT... and it is more pleasant for eyes. similar to snapshot_conf.{c,h} this mimics virsh-snapshot alignment is good here, it is just tabs issue) looks like Makefile.am has ts=8 and this may differ from mail client settings. Anyway this file indentation is rather messy, mixing tabs and spaces... this is fair Name 'tcp' is more appropriate indeed )) I would not use virStorageNetHostTransport and associated structures and xml becasue they have different semantics. In case of backups we have address client can connect to, in case of network disks we have address qemu as client can connect to. And this one has right semantics. I remember I evaluated it as reuse candidate. I thought it was awkward to have port in upper level element: <graphics type='vnc' port='5904' sharePolicy='allow-exclusive'> <listen type='address' address='1.2.3.4'/> </graphics> if we could declare it outdated and recommend to use new style... <graphics type='vnc' sharePolicy='allow-exclusive'> <listen type='address' address='1.2.3.4' port='5904' /> (autoport, tlsPort etc goes here too...) </graphics> Actually we already do this for address attribute: "The address attribute is duplicated as listen attribute in graphics element for backward compatibility. If both are provided they must be equal." So if you are ok with this, i would definetly reuse this element and associated structures. Sorry, this is messy. I should give a different name to attribute that specified full/increment backup in the cover. The code is ok. This attribute has the same meaning as in case of snapshots or disks in domain xml. I just decided not to hurry to declare supported backup options. file + qcow2(probably raw too) is all I have tested. to do or not to do) backup or not to backup this particular disk. The choice is not successful I as see it now, "backup" is better with choices default/off/incremental/full similar to what snapshots have. ouch, copy paste mistake i was talking about future, incremental pull backups are not yet implemented in qemu. So why hurry introducing this element in xml. AFAIK there is not any yet for pull backups. The situation is in case of migration, it is not secure when moving state and disks. This is what Daniel is working on AFAIK. At least to specify where temporary backup files should be. why? element name has no _, to be aligned with snapshots. looks like this name does not do much and again it is copy paste from snapshots, not sure why it has _. But no one needs it now, just don't wanted to introduce dangling code... i'll check, but looks like bool is enough. There is 'migrating' by the way, I can name it 'backup_active' if you prefer. ok, backupdev_created Of course, that is why this comment is here. Can we add the functionality first and then make appropriate refactoring? It is quite a different and probably profound task. It is a pull backup. User don't really need this temporary file. It is keeped only in the process of backup as inverse delta to keep disk state at the moment of backup start. So there is no reason to rename. As to randomly generated names... Do we really need this? We can afford only one backup at a time, one backup delta per disk... You can not check space at all. The size of backup inverse delta heavily depends on guest activity. It can be 0, it can be full disk size. While this is true I thought this can be done in next patches which can address other issues like disk cleanups, handling daemon restart etc. Well I have a copy-paste argument ) IOW snapshots use this already. It comes from b60af444 and here to handle special case when image paths differ only in suffix. Ok, to make some really bulletproof protection we need to make functions like virStorageFileBackendFileCreate to include O_EXCL on open. At least we stay at O(n) complexity. '200 disks' is for real? It is quite ubiquitous to use ! check for undefined/default in libvirt. However this is far from principal, so as you prefer Just future plan )) You mean track nbd server usage? To give better errors or smth? I tried to address failure in code, so that flags are triggered after specific action in the loop, like 'blockdev'. I doubt that splitting the loop will make code more comprehensible. On cleanup you need to undo setup actions in reverse order starting from last successful one which means use 'i' counter after goto and this is more complicated than setup per disk flags in my opinion. Well copy list just to drop 2 line check while structure is mainly the same... good catch Well, no. This function is used by migration code and there is no unix sockets support because migration is always between different hosts. Ok, most of the code use different label instead of flag. IMHO flag is more simple to use, for example when you move a expression you don't need to accomodate label. However to keep the style I'll replace to endjob label. While it is possible to move this check there I think it will blur the function semantics. It quite unexpectable to find this check in the function with such a name. The names says it only fill spaces in backup xml. )) nope, in case QUIESCE is not set frozen is -1 Thank you for review! Nikolay

On 30.09.2016 01:02, John Ferlan wrote: I can split it to API/rpc/driver based stubs/qemu implementation as usual. I just thought all parts except qemu implementation is a kind of boilerplate and do not need much attention... Hmm, this can be an agrgument to split this into 2 parts - boilerplate/qemu implementation. I'd delayed pod until we ready to push upstream, anything interesting will be in a cover letter like just right now but in a less formal form)) This is mimicing snapshots too... Looks like <disks> is redundant yes, but for the sake of 'least surprise' principle I would keep it. True, I had this feeling too to some extent. I'll look deeper into this opportunity in next version. I took simple approach. Stop operation finishes with the error in this case. One don't need complex recovery with backups because they are unobtrusive for user supplied disks. On any error you probably get some garbage in backup targets but user disks are totally untouched. )) True. But hey my goal is stabilize backups API and its significant implementation part. We can hold it from pushing upstream as long as we wish and write missing parts meanwhile. However we deal with upstream and it is really a not-that-stable branch and it is up to user to use some feature or not if is is not complete in all aspects (reality: many start to try then use) It is a test issue actually. Nothing prevents this code work well with large collection of disk from theoretical POV and it is impossible to predict how minizing iterations will influence on this aspect... But I'll give it a try, just not sure what are tradeoffs from number of lines/program structure complexity POV Sounds great! Nikolay

On 05.10.2016 00:55, John Ferlan wrote: But ... I thought it is agreed upon that we'll try to create backup patch in parallell with qemu finishing its part. Even if there will be changes in qemu or in libvirt seems they don't throw away most of the work. The way we find disks size or the way we pass parameters to blockdev-add are quite isolated parts of the patch... Nikolay