On Thu, Aug 10, 2017 at 01:06:27AM +0530, Ishani Chugh wrote: > qemu-backup will be a command-line tool for performing full and > incremental disk backups on running VMs. It is intended as a > reference implementation for management stack and backup developers > to see QEMU's backup features in action. The tool writes details of > guest in a configuration file and the data is retrieved from the file > while creating a backup. The location of config file can be set as an > environment variable QEMU_BACKUP_CONFIG. The usage is as follows: > > Add a guest > python qemu-backup.py guest add --guest <guest_name> --qmp <socket_path> > > Add a drive for backup in a specified guest > python qemu-backup.py drive add --guest <guest_name> --id <drive_id> > [--target <target_file_path>] > > Create backup of the added drives: > python qemu-backup.py backup --guest <guest_name> > > List all guest configs in configuration file: > python qemu-backup.py guest list > > Restore operation > python qemu-backup.py restore --guest <guest-name> > > Remove a guest > python qemu-backup.py guest remove --guest <guest_name> > > > Signed-off-by: Ishani Chugh <chugh.ish...@research.iiit.ac.in> > --- > contrib/backup/qemu-backup.py | 217 > +++++++++++++++++++++++++++--------------- > 1 file changed, 141 insertions(+), 76 deletions(-)
Hi Ishani, This patch is a diff that is based on an existing qemu-backup.py file. The file doesn't exist in qemu.git/master yet so this patch cannot be applied without the missing file. Did you mean to send a new patch series consisting of patches for: 1. qemu-backup.py 2. man page 3. test case ? I suggest using "git rebase -i origin/master" to move your patches onto the latest qemu.git/master and reorder/squash them into a series of logical code changes. > diff --git a/contrib/backup/qemu-backup.py b/contrib/backup/qemu-backup.py > index 9c3dc53..9bbbdb7 100644 > --- a/contrib/backup/qemu-backup.py > +++ b/contrib/backup/qemu-backup.py > @@ -1,22 +1,54 @@ > #!/usr/bin/python > # -*- coding: utf-8 -*- > +# > +# Copyright (C) 2013 Red Hat, Inc. Feel free to add your copyright: Copyright (C) 2017 Ishani Chugh <chugh.ish...@research.iiit.ac.in> > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program 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 General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > +# > + > """ > This file is an implementation of backup tool > """ > +from __future__ import print_function > from argparse import ArgumentParser > import os > import errno > from socket import error as socket_error > -import configparser > +try: > + import configparser > +except ImportError: > + import ConfigParser as configparser > import sys > -sys.path.append('../../scripts/qmp') > +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', > + 'scripts', 'qmp')) > from qmp import QEMUMonitorProtocol > > > class BackupTool(object): > """BackupTool Class""" > - def __init__(self, config_file='backup.ini'): > - self.config_file = config_file > + def __init__(self, > + config_file=os.path.expanduser('~')+'/.qemu/backup/config'): Please use os.path.join() instead of appending strings. You could consider using a variable to avoid repeating this particular path since it is used several times in the code: DEFAULT_CONFIG_FILE = os.path.join(os.path.expanduser('~'), '.qemu', 'backup', 'config') The XDG Base Directory Specification would use ~/.config/qemu instead of ~/.qemu: https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html Modern applications tend to follow this spec. > + if "QEMU_BACKUP_CONFIG" in os.environ: > + self.config_file = os.environ["QEMU_BACKUP_CONFIG"] > + else: > + self.config_file = config_file > + try: > + if not > os.path.isdir(os.path.expanduser('~')+'/.qemu/backup'): os.path.dirname(DEFAULT_CONFIG_FILE) > + os.makedirs(os.path.expanduser('~')+'/.qemu/backup') os.path.dirname(DEFAULT_CONFIG_FILE) > + except: > + print("Cannot find the config file", file=sys.stderr) This error message doesn't match the try-catch block's purpose. The issue was that the config directory couldn't be created. > + exit(1) > self.config = configparser.ConfigParser() > self.config.read(self.config_file) > > @@ -24,66 +56,70 @@ class BackupTool(object): > """ > Writes configuration to ini file. > """ > - with open(self.config_file, 'w') as config_file: > - self.config.write(config_file) > + config_file = open(self.config_file+".tmp", 'w') > + self.config.write(config_file) > + config_file.flush() > + os.fsync(config_file.fileno()) > + config_file.close() > + os.rename(self.config_file+".tmp", self.config_file) > > - def get_socket_path(self, socket_path, tcp): > + def get_socket_address(self, socket_address): > """ > Return Socket address in form of string or tuple > """ > - if tcp is False: > - return os.path.abspath(socket_path) > - return (socket_path.split(':')[0], int(socket_path.split(':')[1])) > + if socket_address.startswith('tcp'): > + return (socket_address.split(':')[1], > + int(socket_address.split(':')[2])) > + return socket_address.split(':',2)[1] > > - def __full_backup(self, guest_name): > + def _full_backup(self, guest_name): > """ > Performs full backup of guest > """ > if guest_name not in self.config.sections(): > - print ("Cannot find specified guest") > - return > - if self.is_guest_running(guest_name, self.config[guest_name]['qmp'], > - self.config[guest_name]['tcp']) is False: > - return > + print ("Cannot find specified guest", file=sys.stderr) print() is a function, there shouldn't be a space before the parentheses: print("message") > + exit(1) > + > + self.verify_guest_running(guest_name) > connection = QEMUMonitorProtocol( > - self.get_socket_path( > - self.config[guest_name]['qmp'], > - self.config[guest_name]['tcp'])) > + self.get_socket_address( > + self.config[guest_name]['qmp'])) > connection.connect() > cmd = {"execute": "transaction", "arguments": {"actions": []}} > for key in self.config[guest_name]: > if key.startswith("drive_"): > - drive = key[key.index('_')+1:] > + drive = key[len('drive_'):] > target = self.config[guest_name][key] > sub_cmd = {"type": "drive-backup", "data": {"device": drive, > "target": target, > "sync": "full"}} > cmd['arguments']['actions'].append(sub_cmd) > - print (connection.cmd_obj(cmd)) > + connection.cmd_obj(cmd) > + if connection.pull_event(wait=True)['event'] == > 'BLOCK_JOB_COMPLETED': > + print("Backup Complete") > + else: > + print("Cannot complete backup", file=sys.stderr) A loop is needed here because innocent QMP events can occur like a VNC client connection. BLOCK_JOB_ERROR is an interesting event to report. Perhaps SHUTDOWN is interesting too. Other than that we need to loop waiting for events and must not exit early.
signature.asc
Description: PGP signature