[libvirt] [RFC] virt-disk : a command line tool for modify domain XML's disk of inactive domains.

Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is. But I want to modify domain XML via commandline tools - for middleware, which modify domains by scripting. - for concsoles, where curses can't work correctly. - for me, I can't remember XML definition detaisl ;) So, I write one. Following script is a script for modify domain XML and allows - add disks - delete disks - show list of disks I think most of elements defined in http://libvirt.org/formatdomain.html#elementsDisks is supported. But I'm an only qemu user and didn't test with Xen and other VMs. What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...) - If not, what is the best way other than making this as my house script ? I'm grad if this is shipped with libvirt is because catching new definition of XML is easy. - Doesn't this one work with your environment ? Example) [root@bluextal pydom]# ./virt-disk.py --domain Guest02 --virtio --source /dev/iscsi_lvm/disk02 Name : vdb Device Type : block Media Type : disk Bus Type : virtio Driver Name : qemu Driver Type : raw Driver Cache : default ReadWrite : ReadWrite Source : /dev/iscsi_lvm/disk02 Address: : AutoFill Add this device Y/N ? y [root@bluextal pydom]# virsh dumpxml Guest02 <domain type='kvm'> ..... <disk type='block' device='disk'> <driver name='qemu' type='raw'/> <source dev='/dev/iscsi_lvm/disk02'/> <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </disk> == Thanks, -Kame == #!/usr/bin/python # # # 'virt-disk' is a command for maintaining disk entities in XML description # of libvirt's VM definition. Default vm assumes 'qemu' # # Copyright (C) 2011 Fujitsu LTD. # # KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> # # Usage: # # Brief help is # %virt-disk --help # # When you want to see list of disks of domain # %virt-disk --domain <domain name> --list # # When you want to remove 'vdb' # %virt-disk --domain <domain name> --delete vdb # # When you want to add block device as virtio block device. # %virt-disk --domain <domain name> --virtio --source /dev/to/mydisk # # When you want to add an IDE cdrom # %virt-disk --domain <domain name> --ide --media cdrom # --source /path/to/ISO.img # # When you want to add a network image as virtio disk. # %virt-disk --domain <domain name> --type network --protocol sheepdog # --host address:port --source resource_name --virtio # # This commad does # # - parse options. # - connect libvirt and get domain xml # - add all devices to disks[] List. # - add all pci address to PCIS[] List # - create new BlockDevice object with regard to options. # - create a XML entry of device # - insert and save it. # import xml.dom import xml.dom.minidom import os import stat import sys import libvirt import re,string from xml.dom import Node from optparse import OptionParser,OptionGroup PCIS = [] disks = [] class Address : pci = [] drive = [] def __init__(self, type) : self.type = type self.domain = None self.bus = None self.slot = None self.function = None self.controller = None self.function = None if type == 'pci' : PCIS.append(self) class BlockDevice : def __init__(self) : self.type = None self.media = None self.driver_cache = None self.driver_name = None self.driver_type = None self.driver_error_policy = None self.driver_io = None self.source = None self.protocol = None self.protocol_addr = None self.protocol_port = None self.target_dev = None self.target_bus = None self.readonly = False self.address_type = None self.address_bus = '' self.address_domain = '' self.address_function = '' self.address_slot = '' self.address_controller= '' self.address_unit='' self.boot_order = None self.shareable = False self.serial = None self.encrypt_format = None self.encrypt_type = None self.encrypt_uuid = None def get_attr(self, name, attr) : x = name.getAttribute(attr) if x == '' : return None return x def find_element(self, ent, name) : for member in ent.childNodes : if member.nodeName == name : return member return None def parse_info(self, name) : # parsing <disk ..... self.type = self.get_attr(name, 'type') self.media = self.get_attr(name, 'device') # parsing <driver .... driver = self.find_element(name, 'driver') if driver != None : self.driver_name = self.get_attr(driver, 'name') if self.driver_name == 'qemu' : self.driver_type = self.get_attr(driver, 'type') self.driver_cache = self.get_attr(driver, 'cache') self.driver_error_policy =\ self.get_attr(driver, 'error_policy') self.driver_io = self.get_attr(driver, 'io') # parsing <source source = self.find_element(name, 'source') if source != None : self.source = self.get_attr(source, 'dev') if self.source == None : self.source = self.get_attr(source, 'file') if self.source == None : self.protocol = self.get_attr(source, 'protocol') self.source = self.get_attr(source, 'name') else : self.source = None # # check protocol and host, port # if self.protocol != None : source = self.find_element(name, 'source') host = self.find_element(source, 'host') if host != None : self.protocol_host = self.get_attr(host, 'host') self.protocol_port = self.get_attr(host, 'port') # parsing <target target = self.find_element(name, 'target') if target != None : self.target_dev = self.get_attr(target, 'dev') self.target_bus = self.get_attr(target, 'bus') # check readonly if self.find_element(name, 'readonly') != None : self.readonly = True # check shareable if self.find_element(name, 'shareable') != None: self.shareable = True # check boot order boot = self.find_element(name, 'boot') if boot != None : self.boot_order = self.get_attr(boot, 'order') # check shareable if self.find_element(name, 'shareable') != None : self.shareable = True # check address address = self.find_element(name, 'address') if address != None : self.address_type = self.get_attr(address, 'type') if self.address_type == 'pci' : self.address_bus = self.get_attr(address, 'bus') self.address_slot = self.get_attr(address, 'slot') self.address_function = self.get_attr(address, 'function') self.address_domain = self.get_attr(address, 'domain') elif self.address_type == 'drive' : self.address_bus = self.get_attr(address, 'bus') self.address_controller = self.get_attr(address, 'controller') self.address_unit = self.get_attr(address, 'unit') # check serial ID serial = self.find_element(name, 'serial') if serial != None : for member in serial.childNodes : if member.nodeType == Node.TEXT_NODE : self.serial = member.data # check encryption encrypt = self.find_element(name, 'encryption') if encrypt != None : self.encrypt_format = self.get_attr(encrypt, 'format') sec = self.find_element(encrypt, 'secret') if sec != None : self.encrypt_type = self.get_attr(sec, 'type') self.encrypt_uuid = self.get_attr(sec, 'uuid') return True def show_ent(self, name, value) : if value != None : print '%-16s:\t%s' %(name, value) def show(self) : self.show_ent('Name', self.target_dev) self.show_ent('Device Type', self.type) self.show_ent('Media Type', self.media) self.show_ent('Bus Type', self.target_bus) self.show_ent('Driver Name', self.driver_name) self.show_ent('Driver Type', self.driver_type) self.show_ent('Driver Cache', self.driver_cache) self.show_ent('Driver I/O', self.driver_io) self.show_ent('Error Policy', self.driver_error_policy) if self.readonly : self.show_ent('ReadWrite', 'ReadOnly') else : self.show_ent('ReadWrite', 'ReadWrite') if self.shareable : self.show_ent('Shareable', 'Yes') self.show_ent('Device Boot Order',self.boot_order) self.show_ent('Protocol', self.protocol) self.show_ent('Hostname', self.protocol_addr) self.show_ent('Port', self.protocol_port) self.show_ent('Source', self.source) if self.address_type == None : self.show_ent('Address:', 'AutoFill') elif self.address_type == 'pci' : x = '%s/%s/%s' %\ (self.address_bus, self.address_slot, self.address_function) self.show_ent('PCI Address', x) elif self.address_type == 'drive' : x = '%s/%s/%s'\ %(self.address_controller, self.address_bus, self.address_unit) self.show_ent('Drive Index', x) if self.serial != None : self.show_ent('Serial', self.serial) if self.encrypt_format != None : self.show_ent('Encryption', self.encrypt_format) self.show_ent(' Type', self.encrypt_type) self.show_ent(' UUID', self.encrypt_uuid) def is_block_device(node) : if node.nodeName == 'disk' : return True return False def parse_address(addr) : type = addr.getAttribute('type') ent = Address(type) if ent.type == 'pci' : ent.bus = addr.getAttribute('bus') ent.slot = addr.getAttribute('slot') ent.function = addr.getAttribute('function') ent.domain = addr.getAttribute('domain') elif ent.type == 'drive' : ent.controller = addr.getAttribute('controller') ent.bus = addr.getAttribute('bus') ent.unit = addr.getAttribute('unit') # Nothing happens return True def check_pci_conflict(bus, slot, function) : sloti = int(slot, 16) for addr in PCIS : if int(addr.slot, 16) == sloti : return True return False def get_free_pci_slot(devname) : for i in range(3, 31) : #qemu allows only 32 slots x = str(i) if not check_pci_conflict('0x00', x, '0x00') : slot = '0x%02x'%(i) return ['0x00', slot, '0x00'] return [] def check_get_free_scsi_slot(index) : addr = scsi_index_to_addr(index) conflict = False for disk in disks : if disk.target_device != scsi: continue if disk.target_controller == addr[0] and\ disk.target_bus == addr[1] and\ disk.target_units == addr[2] : conflict = True break if conflict == False: return addr return [] def check_drive_conflict(type, controller, bus, unit) : for addr in drives : conflict = False for disk in disks : if disk.target_bus != 'type' : continue if disk.address_controller == controller and\ disk.address_bus == bus and\ disk.address_unit == unit : conflict = True break return conflict return True def check_device_conflict(devices, name) : for dev in devices : if dev.target_dev == name : return True return False def gen_scsi_name_from_index(index) : name = 'sd' if index < 26 : name = name + chr(ord('a') + index) elif index < (26 + (26 * 26)) : x = index / 26 - 1 y = index % 26 name = name + chr(ord('a') + x) name = name + chr(ord('a') + y) else : x = (index / 26 - 1) / 26 - 1 y = (index / 26 - 1) % 26 x = index % 26 name = name + chr(ord('a') + x) name = name + chr(ord('a') + y) name = name + chr(ord('a') + z) return name def gen_device_name(devices, head) : if head == 'hd' : for x in 'abcd' : name = 'hd' + x if not check_device_conflict(devices, name) : return name if head == 'vd' : for x in range(0, 26) : name = head + chr(ord('a') + x) if not check_device_conflict(devices, name) : return name if head == 'sd' : for index in range(0, 18278) : name = gen_scsi_name_from_index(index) if not check_device_conflict(devices, name) : return name return None # # Commandline Parser # Using optparse package. Deafault value is below. # usage='usage:%prog --domain Domain <options>....' parser = OptionParser(usage) def help_choice(help, list, default) : help = help + " " help = help + ",".join(list) if (default == True) : help = help + " default: %default" return help parser.add_option('-c', '--connect', action='store', type='string', dest='connect_uri', help='libvirtd connect URI') parser.add_option('-d', '--domain', action='store', type='string',dest="domain", help='domain name.') parser.add_option('-l', '--list', action='store_true', dest='show_list', help='see device list') parser.add_option('--delete', action='store', dest='delete', type='string' , help='remove device') # # 'block' or 'file' can be detected by --source option. # typeChoices=('block','file','network','dir') parser.add_option('--type', choices=typeChoices, dest='type', type='choice', help=help_choice('device type:', typeChoices, False)) mediaChoices=('disk','cdrom') parser.add_option('--media', choices=mediaChoices, type='choice',dest="media", help=help_choice('media type:', mediaChoices, True)) parser.add_option('--source', action='store', type='string', dest='source', help='select source file/device to be shown as disk') protocolChoices=('nbd','rbd', 'sheepdog') parser.add_option('--protocol', choices=protocolChoices, type='choice', dest='protocol', help=help_choice('netdisk protocol:', protocolChoices, False)) parser.add_option('--host', action='store', type='string', dest='host', help='<host:port> for rbd and sheepdog, network devices') parser.add_option('--devname', action='store', type='string', dest='devname', help='device name to be shown to guest(auto at default)') qemu_group = OptionGroup(parser, "Qemu Options") qemuFormatChoices=('raw','bochs','qcow2','qed') qemu_group.add_option('--qemu_disk_format', choices=qemuFormatChoices, type='choice', dest='qemu_format', help=help_choice('disk format(qemu):',qemuFormatChoices, True)) ioChoices=('threads','native') qemu_group.add_option('--io',choices=ioChoices, type='choice', dest='io', help=help_choice('io option(qemu):', ioChoices, False)) cacheChoices=('none','writeback','writethrough','default') qemu_group.add_option('--cache', choices=cacheChoices, type='choice', dest='cache', help=help_choice('cache policy:(qemu)', cacheChoices, True)) errorChoices=('stop','ignore','enospace') parser.add_option('--error_policy', choices=errorChoices, type='choice', dest='error_policy', help=help_choice('error policy:', errorChoices, False)) xen_group = OptionGroup(parser, "Xen Options") xenDriverChoices=('tap','tap2','phy','file') xen_group.add_option('--driver', choices=xenDriverChoices, type='choice', dest='driver_name', help=help_choice('Xen driver type:', xenDriverChoices, False)) xen_group.add_option('--aio', action='store_true', dest='xen_aio', help='using Xen aio driver') parser.add_option('--virtio', action='store_true', dest='virtio', help='create a virtio device') parser.add_option('--readonly', action='store_true', dest='readonly', help='attach device as read only') parser.add_option('--scsi', action='store_true', dest='scsi', help='create a scsi device') parser.add_option('--ide', action='store_true', dest='ide', help='create a ide device') parser.add_option('--pci', action='store', dest='pci', type='string', help='customize pci resource by hand as bus:slot:function') parser.add_option('--drive_id', action='store', dest='drive_id', type='string', help='customize ide/scsi resource ID as controller:bus:unit') parser.add_option('--yes', action='store_true', dest='yes', help='don\'t ask before save') parser.add_option('--nosave', action='store_true', dest='nosave', help='show created device XML instead of saving.') parser.add_option('--boot_order', action='store', dest='boot_order', type='int', help='boot order of the device', metavar='Num') parser.add_option('--shareable', action='store_true', dest='shareable', help='the device can be shareable between device') parser.add_option('--serial', action='store', dest='serial', type='string', help='optional serial ID of the device') EncChoices = ['qcow'] qemu_group.add_option('--encryption_format', choices=EncChoices, type='choice', dest='encryption_format', metavar='qcow', help='Only \"qcow\" is supported') EncTypeChoices =['passphrase'] qemu_group.add_option('--encryption_type', choices=EncTypeChoices, type='choice', dest='encryption_type', help=help_choice('encription_type', EncTypeChoices, True)) qemu_group.add_option('--encryption_uuid', action='store', metavar='UUID', dest='encryption_uuid', help='UUID for encrypted deivce') parser.add_option_group(qemu_group) parser.add_option_group(xen_group) parser.set_defaults(list=False, media='disk') parser.set_defaults(qemu_format='raw', cache='default', media='disk') parser.set_defaults(virtio=False, readonly=False, scsi=False, ide=False) parser.set_defaults(xen_aio=False,) parser.set_defaults(yes=False) (options, args) = parser.parse_args(sys.argv) # # Confirm domain XML is not updated since we opened it. # def check_domain_unchanged(origin, domain): if origin != domain.XMLDesc(0) : print 'Domain file may be updated while we open' print 'please retry' exit(1) # # Connect libvirt and get domain information. # if options.domain == None : print 'please specify domain to be modified' exit(1) try : conn = libvirt.open(options.connect_uri) except: print 'Can\'t connect libvirt with URI %s'%(options.connect_uri) exit(1) conn_uri = conn.getURI() info = re.split(':/', conn_uri) # From the name of connection URI, we detect vmname. vmname = info[0] # domain look up. domain = conn.lookupByName(options.domain) if domain == None: print 'Can\'t find domain %s' %(options.domain) exit(1) # read XML description. origin = domain.XMLDesc(0) dom = xml.dom.minidom.parseString(domain.XMLDesc(0)) # At 1st, we need to find <device> block. names = dom.getElementsByTagName('devices') if (names == None) : print '<device> element not found in %s\n' % filename exit(1) for devices in names : if devices.hasChildNodes() : for name in devices.childNodes : if (is_block_device(name)) : disk = BlockDevice() disk.parse_info(name) disks.append(disk) if name.hasChildNodes() : for elem in name.childNodes : if elem.nodeName == 'address' : addr = parse_address(elem) # # Show List. # if options.show_list == True : for disk in disks : disk.show() print '' exit(0) # delete is easy. if options.delete != None : disk = None for name in devices.childNodes : if (not is_block_device(name)) : continue disk = BlockDevice() disk.parse_info(name) if disk.target_dev == options.delete : devices.removeChild(name) break disk = None if disk == None : print 'no such device %s'%(options.delete) exit(1) if not options.yes : print 'You\'ll delete this device!' disk.show() x = raw_input('Really delete Y/N ? ') if x != 'Y' and x != 'y' : exit(0) str = dom.toxml() check_domain_unchanged(origin, domain) conn.defineXML(str) exit(1) # # Build a newdisk information from command line options # and default values. # newdisk = BlockDevice() if options.type != None : newdisk.type = options.type newdisk.media = options.media # # qemu only has a drive name as 'qemu'. 'type' and 'cache' are selectable. # if vmname == 'qemu' : newdisk.driver_name = 'qemu' newdisk.driver_type = options.qemu_format if options.cache == None : newdisk.driver_cache = 'default' else : newdisk.driver_cache = options.cache else : # Xen can select drive name. newdisk.driver_name = options.driver_name if options.xen_aio == True : newdisk.driver_type = 'aio' if options.error_policy != None : newdisk.driver_error_policy = options.error_policy if options.io != None : newdisk.io = options.io # Make readonly if crdom is specified. if options.media == 'cdrom' or options.readonly == True: newdisk.readonly = True; # Check Device Name and detect device bus from device name. if options.devname != None : if re.match('vd[a-z]', options.devname) : newdisk.target_dev = options.devname newdisk.target_bus = 'virtio' elif re.match('hd[a-d]', options.devname) : newdisk.target_dev = options.devname newdisk.target_bus = 'ide' elif re.match('sd[a-z]', options.devname) : newdisk.target_dev = options.devname newdisk.target_bus = 'scsi' else : newdisk.target_dev = options.devname # # Define device name automatically with regard to the bus. # if options.virtio == True : newdisk.target_bus = 'virtio' if options.devname == None: newdisk.target_dev = gen_device_name(disks, 'vd') if newdisk.target_dev == None: print 'failed to define virtio drive name as vd*' print 'please define by hand with --devname' exit(1) elif options.ide == True : newdisk.target_bus = 'ide' if options.devname == None: newdisk.target_dev = gen_device_name(disks, 'hd') if newdisk.target_dev == None : print 'failed to define ide drive name as hd*' print 'please define by hand with --devname' exit(1) elif options.scsi == True : newdisk.target_bus = 'scsi' if options.devname == None: newdisk.target_dev = gen_device_name(disks, 'sd') if newdisk.target_dev == None : print 'failed to define scsi drive name as sd*' print 'please define by hand with --devname' exit(1) # # If we can't detelct target bus, retrun error. # if newdisk.target_bus == None : print 'need to specify device name or target bus for drive' exit(1) # # If there is a device with the same name, error. # if check_device_conflict(disks, newdisk.target_dev) : print 'device name conflicts %s' %(newdisk.target_dev) print 'please specify an other name with --devname' # # Handle 'source' type. # if options.source != None and options.protocol == None: # No network case. check local FS. # Only absolute path is allowed. if options.source[0] != '/' : print 'please use absolute path for source %s:' %(options.source) exit(1) try: mode = os.stat(options.source)[stat.ST_MODE] except: print 'can\'t handle file %s' %(options.source) exit(1) # # check 'file' and 'block' # newdisk.source = options.source if stat.S_ISREG(mode) != 0: if newdisk.type == None : newdisk.type = 'file' if newdisk.type != 'file' : print '%s specified in --source is file' %(options.source) exit(1) if stat.S_ISBLK(mode) != 0 : if newdisk.type == None : newdisk.type = 'block' if newdisk.type != 'block' : print '%s specified in --source is blkdev' %(options.source) exit(1) if newdisk.type == None : print 'can\'t detect source type %s'%(options.source) elif options.type == 'network' : if options.protocol == None : print 'need to select protocol for network drive.' exit(1) newdisk.protocol = options.protocol if options.source == None : print 'source name for network should be privded --soruce' exit(1) newdisk.source = options.source if not re.match('[0-9a-zA-z\._-]+:[0-9]+', options.host) : print 'host should be specified in address:port manner' exit(1) (addr, port) = re.split(':', options.host) newdisk.protocol_addr = addr newdisk.protocol_port = port if options.media != 'cdrom' and options.source == None : print 'you need to specify source if not cdrom' exit(1) # # Define PCI/IDE/SCSI drive address. # (Usually, all this will be defined automatically.) # # format check if options.pci != None : if options.drive_id != None : print 'pci address and drive-id cannot be set at the same time' exit(1) if not re.match("0x[0-9a-f]+:0x[0-9a-f]+:0x[0-9-a-f]", options.pci) : print 'bad pci address ',options.pci print '0xXX:0xXX:0xXX is expected',options.pci exit(1) if options.drive_id != None : if not re.match("[0-9]+:[0-9]+:[0-9]+", options.drive_id) : print 'bad drive_id address', options.drive_id exit(1) # define BUS ID. # # In this case, device name should meet bus ID(especially IDE), # which the user specified. The user should specify device # name by himself. # if options.pci != None or options.drive_id != None : if options.devname == None : print 'We recommend that --drive_id should be used with --devname', print 'device name <-> drive ID relationship is at random, now' if options.pci != None : pci = re.split(':', options.pci) if '0x00' != pci[0] : print 'only bus 0x00 can be used in pci' exit(1) if check_pci_conflict(pci[0], pci[1], pci[2]) : print 'bus %s conflicts' % (options.pci) newdisk.address_type = 'pci' newdisk.address_bus = pci[0] newdisk.address_slot = pci[1] newdisk.address_function = pci[2] elif options.drive_id != None : drive = re.split(':', options.drive_id) if options.ide == True : if check_drive_conflict('ide', drive[0],drive[1],drive[2]) : print 'drive %s conflicts' % (options.drive_id) else : if check_drive_conflict('scsi',drive[0], drive[1], drive[2]) : print 'drive %s conflicts' % (options.drive_id) newdisk.address_type = 'drive' newdisk.address_controller = drive[0] newdisk.address_bus = drive[1] newdisk.address_unit = drive[2] if options.boot_order != None : newdisk.boot_order = str(options.boot_order) if options.shareable == True: newdisk.shareab;e = True if options.serial != None : newdisk.serial = options.serial # # Handle encryption # if options.encryption_format != None : if options.encryption_type == None or\ options.encryption_uuid == None : print 'encryption passphrase or UUID is not specified' exit(1) newdisk.encrypt_format = options.encryption_format newdisk.encrypt_type = options.encryption_type newdisk.encrypt_uuid = options.encryption_uuid # # Okay, we got all required information. Show the newdisk. # if options.yes != True : newdisk.show() # # Build XML from 'newdisk' # def append_attribute(doc, elem, attr, value) : x = doc.createAttribute(attr) elem.setAttributeNode(x) elem.setAttribute(attr, value) def append_text(doc, elem, text) : x = doc.createTextNode(text) elem.appendChild(x) def append_element(doc, parent, name, level) : append_text(doc, parent, '\n') while level > 0 : append_text(doc, parent, ' ') level = level - 1 element = doc.createElement(name) parent.appendChild(element) return element # <disk .... element = dom.createElement('disk') append_attribute(dom, element, 'device', newdisk.media) append_attribute(dom, element, 'type', newdisk.type) # # <driver ... # child = append_element(dom, element, 'driver', 3) append_attribute(dom, child, 'name', newdisk.driver_name) append_attribute(dom, child, 'type', newdisk.driver_type) if newdisk.driver_cache != None : append_attribute(dom, child, 'cache', newdisk.driver_cache) if newdisk.driver_error_policy != None : append_attribute(dom, child, 'error_policy', newdisk.driver_error_policy) if newdisk.driver_io != None : append_attribute(dom, child, 'io', newdisk.driver_io) # # <source.... # if newdisk.type == 'file' and newdisk.source != None: child = append_element(dom, element, 'source', 3) append_attribute(dom, child, 'file', options.source) elif newdisk.type == 'block' and newdisk.source != None: child = append_element(dom, element, 'source', 3) append_attribute(dom, child, 'dev', options.source) elif newdisk.type == 'network' and newdisk.protocol != None: child = append_element(dom, element, 'source', 3) append_attribute(dom, child, 'protocol', options.protocol) append_attribute(dom, child, 'name', options.source) host = append_element(dom, child, 'host', 4) address = re.split(':',options.host) append_attribute(dom, host, 'name', address[0]) append_attribute(dom, host, 'port', address[1]) # # <target.... # child = append_element(dom, element, 'target', 3) append_attribute(dom, child, 'dev', newdisk.target_dev) append_attribute(dom, child, 'bus', newdisk.target_bus) # # <address..... # libvirt will do auto-fill in typical case. # if newdisk.address_type != None : child = append_element(dom, element, 'address', 3) append_attribute(dom, child, 'type', newdisk.address_type) if newdisk.address_type == 'pci' : append_attribute(dom, child, 'bus', newdisk.address_bus) append_attribute(dom, child, 'slot', newdisk.address_slot) append_attribute(dom, child, 'function', newdisk.address_function) append_attribute(dom, child, 'domain', '0x0000') elif newdisk.address_type == 'drive' : append_attribute(dom, child, 'controller', newdisk.address_controller) append_attribute(dom, child, 'unit', newdisk.address_unit) append_attribute(dom, child, 'bus', newdisk.address_bus) append_attribute(dom, child, 'domain', '0x0000') # # <readonly # if newdisk.readonly == True: append_element(dom, element, 'readonly', 3) # # <shareable # if newdisk.shareable == True: append_element(dom, element, 'readonly', 3) # # <boot # if newdisk.boot_order != None: child = append_element(dom, element, 'boot', 3) append_attribute(dom, child, 'order', newdisk.boot_order) # # <serial # if newdisk.serial != None: child = append_element(dom, element, 'serial', 3) append_text(dom, child, newdisk.serial) # # <encryption # if newdisk.encrypt_format != None : child = append_element(dom, element, 'encryption', 3) append_attribute(dom, child, 'format', newdisk.encrypt_format) secret = append_element(dom, child, 'secret', 4) append_attribute(dom, secret, 'type', newdisk.encrypt_type) append_attribute(dom, secret, 'uuid', newdisk.encrypt_uuid) append_text(dom, child, '\n') # indent for </disk> append_text(dom, element, '\n') append_text(dom, element, ' ') if options.nosave == True : print '' print element.toxml('utf-8') exit(0) # # Insert newdisk to the tail of disks. # for devices in names : if not devices.hasChildNodes() : continue for name in devices.childNodes : if name.nodeName == 'controller' : break if name == None : append_text(dom, devices, ' ') x = dom.createTextNode('\n') devices.appendChild(element) devices.insertBefore(x, name) else: devices.insertBefore(element, name) x = dom.createTextNode('\n ') devices.insertBefore(x, name) if not options.yes : x = raw_input('Add this device Y/N ? ') if x != 'y' and x != 'Y' : exit(1) str = dom.toxml('utf-8') check_domain_unchanged(origin, domain) try: conn.defineXML(str) except: print 'Failed'

On 02/21/2011 03:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
But I want to modify domain XML via commandline tools - for middleware, which modify domains by scripting. - for concsoles, where curses can't work correctly. - for me, I can't remember XML definition detaisl ;)
So, I write one.
Following script is a script for modify domain XML and allows - add disks - delete disks - show list of disks
I think most of elements defined in http://libvirt.org/formatdomain.html#elementsDisks is supported. But I'm an only qemu user and didn't test with Xen and other VMs.
What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...)
- If not, what is the best way other than making this as my house script ? I'm grad if this is shipped with libvirt is because catching new definition of XML is easy.
- Doesn't this one work with your environment ?
Thanks for taking a stab at this, I've been meaning to start a similar tool for some time. However, you should be able to leverage virtinst to accomplish nearly all the XML parsing, and reuse existing virt-install command line options and documentation. Additionally the tool could build or edit any arbitrary domain XML and wouldn't be specific to disks. Take a look at tests/xmlparse.py in the virtinst repo to see how the parsing works and what it's capable of. Thanks, Cole
Example) [root@bluextal pydom]# ./virt-disk.py --domain Guest02 --virtio --source /dev/iscsi_lvm/disk02 Name : vdb Device Type : block Media Type : disk Bus Type : virtio Driver Name : qemu Driver Type : raw Driver Cache : default ReadWrite : ReadWrite Source : /dev/iscsi_lvm/disk02 Address: : AutoFill Add this device Y/N ? y
[root@bluextal pydom]# virsh dumpxml Guest02 <domain type='kvm'> ..... <disk type='block' device='disk'> <driver name='qemu' type='raw'/> <source dev='/dev/iscsi_lvm/disk02'/> <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </disk> ==
Thanks, -Kame
== #!/usr/bin/python # # # 'virt-disk' is a command for maintaining disk entities in XML description # of libvirt's VM definition. Default vm assumes 'qemu' # # Copyright (C) 2011 Fujitsu LTD. # # KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> # # Usage: # # Brief help is # %virt-disk --help # # When you want to see list of disks of domain # %virt-disk --domain <domain name> --list # # When you want to remove 'vdb' # %virt-disk --domain <domain name> --delete vdb # # When you want to add block device as virtio block device. # %virt-disk --domain <domain name> --virtio --source /dev/to/mydisk # # When you want to add an IDE cdrom # %virt-disk --domain <domain name> --ide --media cdrom # --source /path/to/ISO.img # # When you want to add a network image as virtio disk. # %virt-disk --domain <domain name> --type network --protocol sheepdog # --host address:port --source resource_name --virtio # # This commad does # # - parse options. # - connect libvirt and get domain xml # - add all devices to disks[] List. # - add all pci address to PCIS[] List # - create new BlockDevice object with regard to options. # - create a XML entry of device # - insert and save it. #
import xml.dom import xml.dom.minidom import os import stat import sys import libvirt import re,string from xml.dom import Node from optparse import OptionParser,OptionGroup
PCIS = [] disks = []
class Address : pci = [] drive = [] def __init__(self, type) : self.type = type self.domain = None self.bus = None self.slot = None self.function = None self.controller = None self.function = None if type == 'pci' : PCIS.append(self)
class BlockDevice : def __init__(self) : self.type = None self.media = None self.driver_cache = None self.driver_name = None self.driver_type = None self.driver_error_policy = None self.driver_io = None self.source = None self.protocol = None self.protocol_addr = None self.protocol_port = None self.target_dev = None self.target_bus = None self.readonly = False self.address_type = None self.address_bus = '' self.address_domain = '' self.address_function = '' self.address_slot = '' self.address_controller= '' self.address_unit='' self.boot_order = None self.shareable = False self.serial = None self.encrypt_format = None self.encrypt_type = None self.encrypt_uuid = None
def get_attr(self, name, attr) : x = name.getAttribute(attr) if x == '' : return None return x
def find_element(self, ent, name) : for member in ent.childNodes : if member.nodeName == name : return member return None
def parse_info(self, name) : # parsing <disk ..... self.type = self.get_attr(name, 'type') self.media = self.get_attr(name, 'device') # parsing <driver .... driver = self.find_element(name, 'driver') if driver != None : self.driver_name = self.get_attr(driver, 'name') if self.driver_name == 'qemu' : self.driver_type = self.get_attr(driver, 'type') self.driver_cache = self.get_attr(driver, 'cache') self.driver_error_policy =\ self.get_attr(driver, 'error_policy') self.driver_io = self.get_attr(driver, 'io') # parsing <source source = self.find_element(name, 'source') if source != None : self.source = self.get_attr(source, 'dev') if self.source == None : self.source = self.get_attr(source, 'file') if self.source == None : self.protocol = self.get_attr(source, 'protocol') self.source = self.get_attr(source, 'name') else : self.source = None # # check protocol and host, port # if self.protocol != None : source = self.find_element(name, 'source') host = self.find_element(source, 'host') if host != None : self.protocol_host = self.get_attr(host, 'host') self.protocol_port = self.get_attr(host, 'port') # parsing <target target = self.find_element(name, 'target') if target != None : self.target_dev = self.get_attr(target, 'dev') self.target_bus = self.get_attr(target, 'bus') # check readonly if self.find_element(name, 'readonly') != None : self.readonly = True # check shareable if self.find_element(name, 'shareable') != None: self.shareable = True # check boot order boot = self.find_element(name, 'boot') if boot != None : self.boot_order = self.get_attr(boot, 'order') # check shareable if self.find_element(name, 'shareable') != None : self.shareable = True # check address address = self.find_element(name, 'address') if address != None : self.address_type = self.get_attr(address, 'type') if self.address_type == 'pci' : self.address_bus = self.get_attr(address, 'bus') self.address_slot = self.get_attr(address, 'slot') self.address_function = self.get_attr(address, 'function') self.address_domain = self.get_attr(address, 'domain') elif self.address_type == 'drive' : self.address_bus = self.get_attr(address, 'bus') self.address_controller = self.get_attr(address, 'controller') self.address_unit = self.get_attr(address, 'unit') # check serial ID serial = self.find_element(name, 'serial') if serial != None : for member in serial.childNodes : if member.nodeType == Node.TEXT_NODE : self.serial = member.data # check encryption encrypt = self.find_element(name, 'encryption') if encrypt != None : self.encrypt_format = self.get_attr(encrypt, 'format') sec = self.find_element(encrypt, 'secret') if sec != None : self.encrypt_type = self.get_attr(sec, 'type') self.encrypt_uuid = self.get_attr(sec, 'uuid')
return True
def show_ent(self, name, value) : if value != None : print '%-16s:\t%s' %(name, value)
def show(self) : self.show_ent('Name', self.target_dev) self.show_ent('Device Type', self.type) self.show_ent('Media Type', self.media) self.show_ent('Bus Type', self.target_bus) self.show_ent('Driver Name', self.driver_name) self.show_ent('Driver Type', self.driver_type) self.show_ent('Driver Cache', self.driver_cache) self.show_ent('Driver I/O', self.driver_io) self.show_ent('Error Policy', self.driver_error_policy) if self.readonly : self.show_ent('ReadWrite', 'ReadOnly') else : self.show_ent('ReadWrite', 'ReadWrite') if self.shareable : self.show_ent('Shareable', 'Yes') self.show_ent('Device Boot Order',self.boot_order) self.show_ent('Protocol', self.protocol) self.show_ent('Hostname', self.protocol_addr) self.show_ent('Port', self.protocol_port) self.show_ent('Source', self.source)
if self.address_type == None : self.show_ent('Address:', 'AutoFill') elif self.address_type == 'pci' : x = '%s/%s/%s' %\ (self.address_bus, self.address_slot, self.address_function) self.show_ent('PCI Address', x) elif self.address_type == 'drive' : x = '%s/%s/%s'\ %(self.address_controller, self.address_bus, self.address_unit) self.show_ent('Drive Index', x)
if self.serial != None : self.show_ent('Serial', self.serial)
if self.encrypt_format != None : self.show_ent('Encryption', self.encrypt_format) self.show_ent(' Type', self.encrypt_type) self.show_ent(' UUID', self.encrypt_uuid)
def is_block_device(node) : if node.nodeName == 'disk' : return True return False
def parse_address(addr) : type = addr.getAttribute('type') ent = Address(type) if ent.type == 'pci' : ent.bus = addr.getAttribute('bus') ent.slot = addr.getAttribute('slot') ent.function = addr.getAttribute('function') ent.domain = addr.getAttribute('domain') elif ent.type == 'drive' : ent.controller = addr.getAttribute('controller') ent.bus = addr.getAttribute('bus') ent.unit = addr.getAttribute('unit') # Nothing happens return True
def check_pci_conflict(bus, slot, function) : sloti = int(slot, 16) for addr in PCIS : if int(addr.slot, 16) == sloti : return True return False
def get_free_pci_slot(devname) : for i in range(3, 31) : #qemu allows only 32 slots x = str(i) if not check_pci_conflict('0x00', x, '0x00') : slot = '0x%02x'%(i) return ['0x00', slot, '0x00'] return []
def check_get_free_scsi_slot(index) : addr = scsi_index_to_addr(index) conflict = False for disk in disks : if disk.target_device != scsi: continue if disk.target_controller == addr[0] and\ disk.target_bus == addr[1] and\ disk.target_units == addr[2] : conflict = True break if conflict == False: return addr return []
def check_drive_conflict(type, controller, bus, unit) : for addr in drives : conflict = False for disk in disks : if disk.target_bus != 'type' : continue if disk.address_controller == controller and\ disk.address_bus == bus and\ disk.address_unit == unit : conflict = True break return conflict return True
def check_device_conflict(devices, name) : for dev in devices : if dev.target_dev == name : return True return False
def gen_scsi_name_from_index(index) : name = 'sd' if index < 26 : name = name + chr(ord('a') + index) elif index < (26 + (26 * 26)) : x = index / 26 - 1 y = index % 26 name = name + chr(ord('a') + x) name = name + chr(ord('a') + y) else : x = (index / 26 - 1) / 26 - 1 y = (index / 26 - 1) % 26 x = index % 26 name = name + chr(ord('a') + x) name = name + chr(ord('a') + y) name = name + chr(ord('a') + z) return name
def gen_device_name(devices, head) : if head == 'hd' : for x in 'abcd' : name = 'hd' + x if not check_device_conflict(devices, name) : return name if head == 'vd' : for x in range(0, 26) : name = head + chr(ord('a') + x) if not check_device_conflict(devices, name) : return name if head == 'sd' : for index in range(0, 18278) : name = gen_scsi_name_from_index(index) if not check_device_conflict(devices, name) : return name return None
# # Commandline Parser # Using optparse package. Deafault value is below. # usage='usage:%prog --domain Domain <options>....' parser = OptionParser(usage)
def help_choice(help, list, default) : help = help + " " help = help + ",".join(list) if (default == True) : help = help + " default: %default" return help
parser.add_option('-c', '--connect', action='store', type='string', dest='connect_uri', help='libvirtd connect URI')
parser.add_option('-d', '--domain', action='store', type='string',dest="domain", help='domain name.')
parser.add_option('-l', '--list', action='store_true', dest='show_list', help='see device list')
parser.add_option('--delete', action='store', dest='delete', type='string' , help='remove device')
# # 'block' or 'file' can be detected by --source option. # typeChoices=('block','file','network','dir') parser.add_option('--type', choices=typeChoices, dest='type', type='choice', help=help_choice('device type:', typeChoices, False))
mediaChoices=('disk','cdrom') parser.add_option('--media', choices=mediaChoices, type='choice',dest="media", help=help_choice('media type:', mediaChoices, True))
parser.add_option('--source', action='store', type='string', dest='source', help='select source file/device to be shown as disk')
protocolChoices=('nbd','rbd', 'sheepdog') parser.add_option('--protocol', choices=protocolChoices, type='choice', dest='protocol', help=help_choice('netdisk protocol:', protocolChoices, False))
parser.add_option('--host', action='store', type='string', dest='host', help='<host:port> for rbd and sheepdog, network devices')
parser.add_option('--devname', action='store', type='string', dest='devname', help='device name to be shown to guest(auto at default)')
qemu_group = OptionGroup(parser, "Qemu Options") qemuFormatChoices=('raw','bochs','qcow2','qed') qemu_group.add_option('--qemu_disk_format', choices=qemuFormatChoices, type='choice', dest='qemu_format', help=help_choice('disk format(qemu):',qemuFormatChoices, True))
ioChoices=('threads','native') qemu_group.add_option('--io',choices=ioChoices, type='choice', dest='io', help=help_choice('io option(qemu):', ioChoices, False))
cacheChoices=('none','writeback','writethrough','default') qemu_group.add_option('--cache', choices=cacheChoices, type='choice', dest='cache', help=help_choice('cache policy:(qemu)', cacheChoices, True))
errorChoices=('stop','ignore','enospace') parser.add_option('--error_policy', choices=errorChoices, type='choice', dest='error_policy', help=help_choice('error policy:', errorChoices, False))
xen_group = OptionGroup(parser, "Xen Options") xenDriverChoices=('tap','tap2','phy','file') xen_group.add_option('--driver', choices=xenDriverChoices, type='choice', dest='driver_name', help=help_choice('Xen driver type:', xenDriverChoices, False))
xen_group.add_option('--aio', action='store_true', dest='xen_aio', help='using Xen aio driver')
parser.add_option('--virtio', action='store_true', dest='virtio', help='create a virtio device')
parser.add_option('--readonly', action='store_true', dest='readonly', help='attach device as read only')
parser.add_option('--scsi', action='store_true', dest='scsi', help='create a scsi device')
parser.add_option('--ide', action='store_true', dest='ide', help='create a ide device')
parser.add_option('--pci', action='store', dest='pci', type='string', help='customize pci resource by hand as bus:slot:function')
parser.add_option('--drive_id', action='store', dest='drive_id', type='string', help='customize ide/scsi resource ID as controller:bus:unit')
parser.add_option('--yes', action='store_true', dest='yes', help='don\'t ask before save')
parser.add_option('--nosave', action='store_true', dest='nosave', help='show created device XML instead of saving.')
parser.add_option('--boot_order', action='store', dest='boot_order', type='int', help='boot order of the device', metavar='Num')
parser.add_option('--shareable', action='store_true', dest='shareable', help='the device can be shareable between device')
parser.add_option('--serial', action='store', dest='serial', type='string', help='optional serial ID of the device')
EncChoices = ['qcow'] qemu_group.add_option('--encryption_format', choices=EncChoices, type='choice', dest='encryption_format', metavar='qcow', help='Only \"qcow\" is supported')
EncTypeChoices =['passphrase'] qemu_group.add_option('--encryption_type', choices=EncTypeChoices, type='choice', dest='encryption_type', help=help_choice('encription_type', EncTypeChoices, True))
qemu_group.add_option('--encryption_uuid', action='store', metavar='UUID', dest='encryption_uuid', help='UUID for encrypted deivce')
parser.add_option_group(qemu_group) parser.add_option_group(xen_group) parser.set_defaults(list=False, media='disk') parser.set_defaults(qemu_format='raw', cache='default', media='disk') parser.set_defaults(virtio=False, readonly=False, scsi=False, ide=False) parser.set_defaults(xen_aio=False,) parser.set_defaults(yes=False)
(options, args) = parser.parse_args(sys.argv)
# # Confirm domain XML is not updated since we opened it. # def check_domain_unchanged(origin, domain): if origin != domain.XMLDesc(0) : print 'Domain file may be updated while we open' print 'please retry' exit(1) # # Connect libvirt and get domain information. # if options.domain == None : print 'please specify domain to be modified' exit(1)
try : conn = libvirt.open(options.connect_uri) except: print 'Can\'t connect libvirt with URI %s'%(options.connect_uri) exit(1)
conn_uri = conn.getURI() info = re.split(':/', conn_uri)
# From the name of connection URI, we detect vmname. vmname = info[0]
# domain look up. domain = conn.lookupByName(options.domain) if domain == None: print 'Can\'t find domain %s' %(options.domain) exit(1)
# read XML description. origin = domain.XMLDesc(0) dom = xml.dom.minidom.parseString(domain.XMLDesc(0))
# At 1st, we need to find <device> block. names = dom.getElementsByTagName('devices') if (names == None) : print '<device> element not found in %s\n' % filename exit(1)
for devices in names : if devices.hasChildNodes() : for name in devices.childNodes : if (is_block_device(name)) : disk = BlockDevice() disk.parse_info(name) disks.append(disk) if name.hasChildNodes() : for elem in name.childNodes : if elem.nodeName == 'address' : addr = parse_address(elem)
# # Show List. # if options.show_list == True : for disk in disks : disk.show() print '' exit(0)
# delete is easy. if options.delete != None : disk = None for name in devices.childNodes : if (not is_block_device(name)) : continue disk = BlockDevice() disk.parse_info(name) if disk.target_dev == options.delete : devices.removeChild(name) break disk = None if disk == None : print 'no such device %s'%(options.delete) exit(1) if not options.yes : print 'You\'ll delete this device!' disk.show() x = raw_input('Really delete Y/N ? ') if x != 'Y' and x != 'y' : exit(0) str = dom.toxml() check_domain_unchanged(origin, domain) conn.defineXML(str) exit(1)
# # Build a newdisk information from command line options # and default values. # newdisk = BlockDevice() if options.type != None : newdisk.type = options.type
newdisk.media = options.media
# # qemu only has a drive name as 'qemu'. 'type' and 'cache' are selectable. # if vmname == 'qemu' : newdisk.driver_name = 'qemu' newdisk.driver_type = options.qemu_format if options.cache == None : newdisk.driver_cache = 'default' else : newdisk.driver_cache = options.cache else : # Xen can select drive name. newdisk.driver_name = options.driver_name if options.xen_aio == True : newdisk.driver_type = 'aio'
if options.error_policy != None : newdisk.driver_error_policy = options.error_policy
if options.io != None : newdisk.io = options.io
# Make readonly if crdom is specified. if options.media == 'cdrom' or options.readonly == True: newdisk.readonly = True;
# Check Device Name and detect device bus from device name. if options.devname != None : if re.match('vd[a-z]', options.devname) : newdisk.target_dev = options.devname newdisk.target_bus = 'virtio' elif re.match('hd[a-d]', options.devname) : newdisk.target_dev = options.devname newdisk.target_bus = 'ide' elif re.match('sd[a-z]', options.devname) : newdisk.target_dev = options.devname newdisk.target_bus = 'scsi' else : newdisk.target_dev = options.devname # # Define device name automatically with regard to the bus. # if options.virtio == True : newdisk.target_bus = 'virtio' if options.devname == None: newdisk.target_dev = gen_device_name(disks, 'vd') if newdisk.target_dev == None: print 'failed to define virtio drive name as vd*' print 'please define by hand with --devname' exit(1) elif options.ide == True : newdisk.target_bus = 'ide' if options.devname == None: newdisk.target_dev = gen_device_name(disks, 'hd') if newdisk.target_dev == None : print 'failed to define ide drive name as hd*' print 'please define by hand with --devname' exit(1)
elif options.scsi == True : newdisk.target_bus = 'scsi' if options.devname == None: newdisk.target_dev = gen_device_name(disks, 'sd') if newdisk.target_dev == None : print 'failed to define scsi drive name as sd*' print 'please define by hand with --devname' exit(1) # # If we can't detelct target bus, retrun error. # if newdisk.target_bus == None : print 'need to specify device name or target bus for drive' exit(1)
# # If there is a device with the same name, error. # if check_device_conflict(disks, newdisk.target_dev) : print 'device name conflicts %s' %(newdisk.target_dev) print 'please specify an other name with --devname'
# # Handle 'source' type. # if options.source != None and options.protocol == None: # No network case. check local FS. # Only absolute path is allowed. if options.source[0] != '/' : print 'please use absolute path for source %s:' %(options.source) exit(1) try: mode = os.stat(options.source)[stat.ST_MODE] except: print 'can\'t handle file %s' %(options.source) exit(1) # # check 'file' and 'block' # newdisk.source = options.source if stat.S_ISREG(mode) != 0: if newdisk.type == None : newdisk.type = 'file' if newdisk.type != 'file' : print '%s specified in --source is file' %(options.source) exit(1) if stat.S_ISBLK(mode) != 0 : if newdisk.type == None : newdisk.type = 'block' if newdisk.type != 'block' : print '%s specified in --source is blkdev' %(options.source) exit(1) if newdisk.type == None : print 'can\'t detect source type %s'%(options.source)
elif options.type == 'network' : if options.protocol == None : print 'need to select protocol for network drive.' exit(1) newdisk.protocol = options.protocol if options.source == None : print 'source name for network should be privded --soruce' exit(1) newdisk.source = options.source if not re.match('[0-9a-zA-z\._-]+:[0-9]+', options.host) : print 'host should be specified in address:port manner' exit(1) (addr, port) = re.split(':', options.host) newdisk.protocol_addr = addr newdisk.protocol_port = port
if options.media != 'cdrom' and options.source == None : print 'you need to specify source if not cdrom' exit(1)
# # Define PCI/IDE/SCSI drive address. # (Usually, all this will be defined automatically.) #
# format check if options.pci != None : if options.drive_id != None : print 'pci address and drive-id cannot be set at the same time' exit(1) if not re.match("0x[0-9a-f]+:0x[0-9a-f]+:0x[0-9-a-f]", options.pci) : print 'bad pci address ',options.pci print '0xXX:0xXX:0xXX is expected',options.pci exit(1)
if options.drive_id != None : if not re.match("[0-9]+:[0-9]+:[0-9]+", options.drive_id) : print 'bad drive_id address', options.drive_id exit(1)
# define BUS ID. # # In this case, device name should meet bus ID(especially IDE), # which the user specified. The user should specify device # name by himself. # if options.pci != None or options.drive_id != None : if options.devname == None : print 'We recommend that --drive_id should be used with --devname', print 'device name <-> drive ID relationship is at random, now'
if options.pci != None : pci = re.split(':', options.pci) if '0x00' != pci[0] : print 'only bus 0x00 can be used in pci' exit(1) if check_pci_conflict(pci[0], pci[1], pci[2]) : print 'bus %s conflicts' % (options.pci) newdisk.address_type = 'pci' newdisk.address_bus = pci[0] newdisk.address_slot = pci[1] newdisk.address_function = pci[2]
elif options.drive_id != None : drive = re.split(':', options.drive_id)
if options.ide == True : if check_drive_conflict('ide', drive[0],drive[1],drive[2]) : print 'drive %s conflicts' % (options.drive_id) else : if check_drive_conflict('scsi',drive[0], drive[1], drive[2]) : print 'drive %s conflicts' % (options.drive_id)
newdisk.address_type = 'drive' newdisk.address_controller = drive[0] newdisk.address_bus = drive[1] newdisk.address_unit = drive[2]
if options.boot_order != None : newdisk.boot_order = str(options.boot_order)
if options.shareable == True: newdisk.shareab;e = True
if options.serial != None : newdisk.serial = options.serial # # Handle encryption #
if options.encryption_format != None : if options.encryption_type == None or\ options.encryption_uuid == None : print 'encryption passphrase or UUID is not specified' exit(1) newdisk.encrypt_format = options.encryption_format newdisk.encrypt_type = options.encryption_type newdisk.encrypt_uuid = options.encryption_uuid
# # Okay, we got all required information. Show the newdisk. # if options.yes != True : newdisk.show() # # Build XML from 'newdisk' # def append_attribute(doc, elem, attr, value) : x = doc.createAttribute(attr) elem.setAttributeNode(x) elem.setAttribute(attr, value)
def append_text(doc, elem, text) : x = doc.createTextNode(text) elem.appendChild(x)
def append_element(doc, parent, name, level) : append_text(doc, parent, '\n') while level > 0 : append_text(doc, parent, ' ') level = level - 1 element = doc.createElement(name) parent.appendChild(element) return element
# <disk .... element = dom.createElement('disk') append_attribute(dom, element, 'device', newdisk.media) append_attribute(dom, element, 'type', newdisk.type)
# # <driver ... # child = append_element(dom, element, 'driver', 3) append_attribute(dom, child, 'name', newdisk.driver_name) append_attribute(dom, child, 'type', newdisk.driver_type) if newdisk.driver_cache != None : append_attribute(dom, child, 'cache', newdisk.driver_cache) if newdisk.driver_error_policy != None : append_attribute(dom, child, 'error_policy', newdisk.driver_error_policy) if newdisk.driver_io != None : append_attribute(dom, child, 'io', newdisk.driver_io)
# # <source.... # if newdisk.type == 'file' and newdisk.source != None: child = append_element(dom, element, 'source', 3) append_attribute(dom, child, 'file', options.source) elif newdisk.type == 'block' and newdisk.source != None: child = append_element(dom, element, 'source', 3) append_attribute(dom, child, 'dev', options.source) elif newdisk.type == 'network' and newdisk.protocol != None: child = append_element(dom, element, 'source', 3) append_attribute(dom, child, 'protocol', options.protocol) append_attribute(dom, child, 'name', options.source) host = append_element(dom, child, 'host', 4) address = re.split(':',options.host) append_attribute(dom, host, 'name', address[0]) append_attribute(dom, host, 'port', address[1]) # # <target.... # child = append_element(dom, element, 'target', 3) append_attribute(dom, child, 'dev', newdisk.target_dev) append_attribute(dom, child, 'bus', newdisk.target_bus)
# # <address..... # libvirt will do auto-fill in typical case. # if newdisk.address_type != None : child = append_element(dom, element, 'address', 3) append_attribute(dom, child, 'type', newdisk.address_type)
if newdisk.address_type == 'pci' : append_attribute(dom, child, 'bus', newdisk.address_bus) append_attribute(dom, child, 'slot', newdisk.address_slot) append_attribute(dom, child, 'function', newdisk.address_function) append_attribute(dom, child, 'domain', '0x0000')
elif newdisk.address_type == 'drive' : append_attribute(dom, child, 'controller', newdisk.address_controller) append_attribute(dom, child, 'unit', newdisk.address_unit) append_attribute(dom, child, 'bus', newdisk.address_bus) append_attribute(dom, child, 'domain', '0x0000') # # <readonly # if newdisk.readonly == True: append_element(dom, element, 'readonly', 3)
# # <shareable # if newdisk.shareable == True: append_element(dom, element, 'readonly', 3) # # <boot # if newdisk.boot_order != None: child = append_element(dom, element, 'boot', 3) append_attribute(dom, child, 'order', newdisk.boot_order)
# # <serial # if newdisk.serial != None: child = append_element(dom, element, 'serial', 3) append_text(dom, child, newdisk.serial)
# # <encryption #
if newdisk.encrypt_format != None : child = append_element(dom, element, 'encryption', 3) append_attribute(dom, child, 'format', newdisk.encrypt_format) secret = append_element(dom, child, 'secret', 4) append_attribute(dom, secret, 'type', newdisk.encrypt_type) append_attribute(dom, secret, 'uuid', newdisk.encrypt_uuid) append_text(dom, child, '\n')
# indent for </disk> append_text(dom, element, '\n') append_text(dom, element, ' ')
if options.nosave == True : print '' print element.toxml('utf-8') exit(0)
# # Insert newdisk to the tail of disks. # for devices in names : if not devices.hasChildNodes() : continue for name in devices.childNodes : if name.nodeName == 'controller' : break if name == None : append_text(dom, devices, ' ') x = dom.createTextNode('\n') devices.appendChild(element) devices.insertBefore(x, name) else: devices.insertBefore(element, name) x = dom.createTextNode('\n ') devices.insertBefore(x, name)
if not options.yes : x = raw_input('Add this device Y/N ? ') if x != 'y' and x != 'Y' : exit(1)
str = dom.toxml('utf-8')
check_domain_unchanged(origin, domain)
try: conn.defineXML(str) except: print 'Failed'
_______________________________________________ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list

On Mon, 21 Feb 2011 10:08:46 -0500 Cole Robinson <crobinso@redhat.com> wrote:
On 02/21/2011 03:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
But I want to modify domain XML via commandline tools - for middleware, which modify domains by scripting. - for concsoles, where curses can't work correctly. - for me, I can't remember XML definition detaisl ;)
So, I write one.
Following script is a script for modify domain XML and allows - add disks - delete disks - show list of disks
I think most of elements defined in http://libvirt.org/formatdomain.html#elementsDisks is supported. But I'm an only qemu user and didn't test with Xen and other VMs.
What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...)
- If not, what is the best way other than making this as my house script ? I'm grad if this is shipped with libvirt is because catching new definition of XML is easy.
- Doesn't this one work with your environment ?
Thanks for taking a stab at this, I've been meaning to start a similar tool for some time. However, you should be able to leverage virtinst to accomplish nearly all the XML parsing, and reuse existing virt-install command line options and documentation. Additionally the tool could build or edit any arbitrary domain XML and wouldn't be specific to disks.
Thank you, that's the information I wanted to hear ...where this kind of command should be packaged into. I'll look virt-inst package. And, as I wrote, this is just an example, we'll support all cpu,memory,interface,usb etc.... Do you think should all be supported by 'a' command ? or by a set of commands ? [at modify cpu] % virt-modify --cpu ..... or % virt-cpu-modify .....
Take a look at tests/xmlparse.py in the virtinst repo to see how the parsing works and what it's capable of.
Thanks. We'll look into. Regards, -Kame

On 02/21/2011 04:48 PM, KAMEZAWA Hiroyuki wrote:
What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...)
What's wrong with directly patch libvirt's virsh command line tool to add in the extra functionality?
Do you think should all be supported by 'a' command ? or by a set of commands ?
[at modify cpu] % virt-modify --cpu ..... or % virt-cpu-modify .....
That is, for the example of modifying the number of vcpus available to a guest, what's wrong with 'virsh setvcpus' with its various options? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Mon, 21 Feb 2011 17:15:21 -0700 Eric Blake <eblake@redhat.com> wrote:
On 02/21/2011 04:48 PM, KAMEZAWA Hiroyuki wrote:
What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...)
What's wrong with directly patch libvirt's virsh command line tool to add in the extra functionality?
Just because I got a impression that 'there is a design policy that libvirt should work only with active domain' after talk with redhat guys. If it's allowed, it's best.
Do you think should all be supported by 'a' command ? or by a set of commands ?
[at modify cpu] % virt-modify --cpu ..... or % virt-cpu-modify .....
That is, for the example of modifying the number of vcpus available to a guest, what's wrong with 'virsh setvcpus' with its various options?
No problem with that. Off topic) BTW, with %virsh vcpupin --domain ID --vcpu 0 --cpulist 0 %virsh vcpupin --domain ID --vcpu 1 --cpulist 1 vcpu '0' will be on '0' and vcpu '1' will be on '1'. How to describe this in XML format ? <vcpu cpuset="0-1">2</vcpu> will allow vcpu0 onto 0 or 1, vcpu1 onto 0 or 1. Right ? Thanks, -Kame

On 02/21/2011 07:15 PM, Eric Blake wrote:
On 02/21/2011 04:48 PM, KAMEZAWA Hiroyuki wrote:
What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...)
What's wrong with directly patch libvirt's virsh command line tool to add in the extra functionality?
Do you think should all be supported by 'a' command ? or by a set of commands ?
[at modify cpu] % virt-modify --cpu ..... or % virt-cpu-modify .....
That is, for the example of modifying the number of vcpus available to a guest, what's wrong with 'virsh setvcpus' with its various options?
I think --cpu was means <cpu> XML, not <vpcu> XML. Generally I don't think we should patch virsh for this because the overhead of implementing this all in C is prohibitive compared to python. Additionally there is already an external library that implements the vast majority of libvirt XML building and parsing (virtinst). Granted if we allowed virsh access to src/conf/*, we would get XML handling for free, but it would still require lots of command line handling, which would be duplicating functionality that already needs to live in virtinst/virt-manager anyways. I think a new tool is required and that it would ideally supersede virsh for all XML editing/creating. - Cole

On 02/21/2011 06:48 PM, KAMEZAWA Hiroyuki wrote:
On Mon, 21 Feb 2011 10:08:46 -0500 Cole Robinson <crobinso@redhat.com> wrote:
On 02/21/2011 03:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
But I want to modify domain XML via commandline tools - for middleware, which modify domains by scripting. - for concsoles, where curses can't work correctly. - for me, I can't remember XML definition detaisl ;)
So, I write one.
Following script is a script for modify domain XML and allows - add disks - delete disks - show list of disks
I think most of elements defined in http://libvirt.org/formatdomain.html#elementsDisks is supported. But I'm an only qemu user and didn't test with Xen and other VMs.
What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...)
- If not, what is the best way other than making this as my house script ? I'm grad if this is shipped with libvirt is because catching new definition of XML is easy.
- Doesn't this one work with your environment ?
Thanks for taking a stab at this, I've been meaning to start a similar tool for some time. However, you should be able to leverage virtinst to accomplish nearly all the XML parsing, and reuse existing virt-install command line options and documentation. Additionally the tool could build or edit any arbitrary domain XML and wouldn't be specific to disks.
Thank you, that's the information I wanted to hear ...where this kind of command should be packaged into. I'll look virt-inst package. And, as I wrote, this is just an example, we'll support all cpu,memory,interface,usb etc....
Do you think should all be supported by 'a' command ? or by a set of commands ?
[at modify cpu] % virt-modify --cpu ..... or % virt-cpu-modify .....
I would say just a single command. I imagined it would be called virt-xml and would be able to do the following: - Lookup an existing libvirt object and edit inactive XML in place - Lookup an existing libvirt object and perform hotplug operations - Take an XML document from a file or stdin and edit XML - Generate domain device XML, possibly also pool, vol, network, interface, etc. XML Maybe we should have separate tools for domain vs. storage vs. network etc, but I think the first iteration of the tool will be domain specific anyways so we can defer the question for a bit. Thanks, Cole
Take a look at tests/xmlparse.py in the virtinst repo to see how the parsing works and what it's capable of.
Thanks. We'll look into.
Regards, -Kame

On Tue, 22 Feb 2011 07:36:18 -0500 Cole Robinson <crobinso@redhat.com> wrote:
On 02/21/2011 06:48 PM, KAMEZAWA Hiroyuki wrote:
On Mon, 21 Feb 2011 10:08:46 -0500 Cole Robinson <crobinso@redhat.com> wrote:
On 02/21/2011 03:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
But I want to modify domain XML via commandline tools - for middleware, which modify domains by scripting. - for concsoles, where curses can't work correctly. - for me, I can't remember XML definition detaisl ;)
So, I write one.
Following script is a script for modify domain XML and allows - add disks - delete disks - show list of disks
I think most of elements defined in http://libvirt.org/formatdomain.html#elementsDisks is supported. But I'm an only qemu user and didn't test with Xen and other VMs.
What I wanted to hear opinions as 'RFC' is - Can this be shipped with libvirt as one of a tool ? (with more documents) (If so, we'll write other scripts for cpu,network,memory,etc...)
- If not, what is the best way other than making this as my house script ? I'm grad if this is shipped with libvirt is because catching new definition of XML is easy.
- Doesn't this one work with your environment ?
Thanks for taking a stab at this, I've been meaning to start a similar tool for some time. However, you should be able to leverage virtinst to accomplish nearly all the XML parsing, and reuse existing virt-install command line options and documentation. Additionally the tool could build or edit any arbitrary domain XML and wouldn't be specific to disks.
Thank you, that's the information I wanted to hear ...where this kind of command should be packaged into. I'll look virt-inst package. And, as I wrote, this is just an example, we'll support all cpu,memory,interface,usb etc....
Do you think should all be supported by 'a' command ? or by a set of commands ?
[at modify cpu] % virt-modify --cpu ..... or % virt-cpu-modify .....
I would say just a single command. I imagined it would be called virt-xml and would be able to do the following:
- Lookup an existing libvirt object and edit inactive XML in place - Lookup an existing libvirt object and perform hotplug operations - Take an XML document from a file or stdin and edit XML - Generate domain device XML, possibly also pool, vol, network, interface, etc. XML
Maybe we should have separate tools for domain vs. storage vs. network etc, but I think the first iteration of the tool will be domain specific anyways so we can defer the question for a bit.
For me, the 1st choice is supporting virsh attach-disk because our customers will move from Xen to KVM. The reason customer moves from Xen to KVM is just because they'll move from RHEL5 to RHEL6. Considering moving from RHEL5 to RHEL6, 'virsh-detach disk doesn't work as RHEL5' is a regression. So, we'd like to fix virsh-attach/detach-disk. please allow. (Customers don't are what is the bundled VM, just take care of commands they used works.) But yes, I think doing 'full support' of XML in virsh.c will make the code dirty commands complex. Hmm, but, after a brief hack, it seems libvirt's /conf codes does enough jobs. I'll try 'virsh' 1st. Thanks, -Kame

On 02/21/2011 01:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
Actually, libvirt should be patched to learn how to modify xml of inactive disks for qemu (it already can do it for xen, so the API is already present, it's just that no one has wired up that API for qemu). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Mon, 21 Feb 2011 08:17:30 -0700 Eric Blake <eblake@redhat.com> wrote:
On 02/21/2011 01:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
Actually, libvirt should be patched to learn how to modify xml of inactive disks for qemu (it already can do it for xen, so the API is already present, it's just that no one has wired up that API for qemu).
Before starging this, I thought of that. I did this in python by 3 reasons. 1. When we asked "Is it a spec that we cannot modify inactive domain ?" to a Redhat guy, he answered "it's a spec". Do you, maintainers, have some concensus about this ? 2. virsh attach-disk doesn't seem to support misc. options. It doesn't have - boot_order - shareable - serial - io - error_policy etc... So, I thought independent python script will be easier and will help keeping virsh/libvirt clean. 3. I wanted an automatic XML genarator for virsh attach-device. So, I thought I should start from a python script. It can be used for a XML generator for virsh attach-device even if libvirt finally support XML modification. Regards, -Kame

On 02/21/2011 05:04 PM, KAMEZAWA Hiroyuki wrote:
On Mon, 21 Feb 2011 08:17:30 -0700 Eric Blake <eblake@redhat.com> wrote:
On 02/21/2011 01:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
Actually, libvirt should be patched to learn how to modify xml of inactive disks for qemu (it already can do it for xen, so the API is already present, it's just that no one has wired up that API for qemu).
Before starging this, I thought of that. I did this in python by 3 reasons.
1. When we asked "Is it a spec that we cannot modify inactive domain ?" to a Redhat guy, he answered "it's a spec". Do you, maintainers, have some concensus about this ?
'virsh attach-disk --persistent' is supposed to be able to modify an inactive domain. If it doesn't do so for qemu, then that's because no one has yet implemented it correctly, which means libvirt has a bug that needs to be patched. For example, see: https://bugzilla.redhat.com/show_bug.cgi?id=669549 about 'virsh setmem --config' not working for qemu.
2. virsh attach-disk doesn't seem to support misc. options. It doesn't have - boot_order - shareable - serial - io - error_policy etc...
If there's something that the libvirt API supports, but which virsh does not support, then that's a bug in virsh. Please let us know about these usability deficiencies in virsh, since that is the right place to be patching it for use by all other shell-based tools, rather than reinventing a new tool by every user. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 02/21/2011 05:20 PM, Eric Blake wrote:
1. When we asked "Is it a spec that we cannot modify inactive domain ?" to a Redhat guy, he answered "it's a spec". Do you, maintainers, have some concensus about this ?
'virsh attach-disk --persistent' is supposed to be able to modify an inactive domain. If it doesn't do so for qemu, then that's because no one has yet implemented it correctly, which means libvirt has a bug that needs to be patched. For example, see:
https://bugzilla.redhat.com/show_bug.cgi?id=669549
about 'virsh setmem --config' not working for qemu.
And https://bugzilla.redhat.com/show_bug.cgi?id=658713 for attach-disk --persistent. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Mon, 21 Feb 2011 17:27:13 -0700 Eric Blake <eblake@redhat.com> wrote:
On 02/21/2011 05:20 PM, Eric Blake wrote:
1. When we asked "Is it a spec that we cannot modify inactive domain ?" to a Redhat guy, he answered "it's a spec". Do you, maintainers, have some concensus about this ?
'virsh attach-disk --persistent' is supposed to be able to modify an inactive domain. If it doesn't do so for qemu, then that's because no one has yet implemented it correctly, which means libvirt has a bug that needs to be patched. For example, see:
https://bugzilla.redhat.com/show_bug.cgi?id=669549
about 'virsh setmem --config' not working for qemu.
And https://bugzilla.redhat.com/show_bug.cgi?id=658713 for attach-disk --persistent.
This is a quick hack aginst attach-disk. Seems to work fine for me. But, for example, 'What lock should be held ?' is not very clear to me ;) CC'ed to my co-workers for sharing. Please point out bad things you notice. Total update with detach-disk and attach/detach interfaces will be posted. ==
From 14e1de99d433ba68917a80cc5b82da7f9436d419 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa@bluextal.(none)> Date: Tue, 22 Feb 2011 16:09:21 +0900 Subject: [PATCH] This patch is for allowing inactive domain by virsh command.
As the first step, this just only supports attach-disk. Further update will allow to support other commands. After this patch, % virsh attach-disk ... --persistent will modify domain XML format of inactive domain. For now, --persistent against active domain returns error. Note: At this stage, I can't guarantee the change will be applied against both of alive guest and its XML definition in atomic. And this is the same behavior Xen driver does. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/qemu/qemu_driver.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0f25a2a..80cb724 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4082,16 +4082,125 @@ cleanup: return ret; } +/* + * Attach a device given by XML, the change will be persistent + * and domain XML definition file is updated. + */ +static int qemuDomainAttachDevicePersistent(virDomainPtr dom, + const char *xml) +{ + struct qemud_driver *driver; + virDomainDeviceDefPtr newdev; + virDomainDefPtr vmdef; + virDomainObjPtr vm; + int i, ret = -1; + int bus_update = -1; + + if ((!dom) || (!dom->conn) || (!dom->name) || (!xml)) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("internal error : %s"), __FUNCTION__); + return -1; + } + + if (dom->conn->flags & VIR_CONNECT_RO) + return -1; + + driver = dom->conn->privateData; + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("cannot find domain '%s'"), dom->name); + goto unlock_out; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + goto unlock_out; + + if (virDomainObjIsActive(vm)) { + /* + * For now, just allow updating inactive domains. Further development + * will allow updating both active domain and its config file at + * the same time. + */ + qemuReportError(VIR_ERR_INVALID_ARG, + _("cannot update alive domain : %s"), __FUNCTION__); + goto endjob; + } + + if (vm->newDef) + vmdef = vm->newDef; + else + vmdef = vm->def; + + newdev = virDomainDeviceDefParse(driver->caps, + vmdef, xml, VIR_DOMAIN_XML_INACTIVE); + if (!newdev) + goto endjob; + /* At first, check device confliction */ + switch(newdev->type) { + case VIR_DOMAIN_DEVICE_DISK: + for (i = 0; i < vmdef->ndisks; i++) { + virDomainDiskDefPtr vdisk; + //virDomainDiskDefPtr vdisk; + vdisk = vm->def->disks[i]; + if (strcmp(vdisk->dst, newdev->data.disk->dst) == 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), vdisk->dst); + goto endjob; + } + } + if (virDomainDiskInsert(vmdef, newdev->data.disk)) { + virReportOOMError(); + goto endjob; + } + qemuReportError(VIR_ERR_INTERNAL_ERROR, "address info %p", + newdev->data.disk->info); + if (newdev->data.disk->info == NULL) { + /* Auto generate PCI address */ + if (newdev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) + qemuDomainAssignPCIAddresses(vmdef); + /* Other devices as IDE, SCSI...will get ID automatically */ + } + newdev->data.disk = NULL; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Sorry, the device is not suppored for now")); + goto endjob; + } + + ret = virDomainSaveConfig(driver->configDir, vmdef); + +endjob: + if (qemuDomainObjEndJob(vm) == 0) + vm = NULL; + if (vm) + virDomainObjUnlock(vm); + /* Note: insert of newdev is done by copy */ + virDomainDeviceDefFree(newdev); +unlock_out: + qemuDriverUnlock(driver); + return ret; +} + static int qemudDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot modify the persistent configuration of a domain")); - return -1; + /* + * Because we can't update the live guest and XML + * in atomic, limiting modification as only-acrive and + * only-inactive. Need some idea to update both at the same time. + */ + return qemuDomainAttachDevicePersistent(dom, xml); } - return qemudDomainAttachDevice(dom, xml); + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) + return qemudDomainAttachDevice(dom, xml); + + return -1; } -- 1.7.1

On Mon, 21 Feb 2011 17:20:56 -0700 Eric Blake <eblake@redhat.com> wrote:
On 02/21/2011 05:04 PM, KAMEZAWA Hiroyuki wrote:
On Mon, 21 Feb 2011 08:17:30 -0700 Eric Blake <eblake@redhat.com> wrote:
On 02/21/2011 01:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
Actually, libvirt should be patched to learn how to modify xml of inactive disks for qemu (it already can do it for xen, so the API is already present, it's just that no one has wired up that API for qemu).
Before starging this, I thought of that. I did this in python by 3 reasons.
1. When we asked "Is it a spec that we cannot modify inactive domain ?" to a Redhat guy, he answered "it's a spec". Do you, maintainers, have some concensus about this ?
'virsh attach-disk --persistent' is supposed to be able to modify an inactive domain. If it doesn't do so for qemu, then that's because no one has yet implemented it correctly, which means libvirt has a bug that needs to be patched. For example, see:
https://bugzilla.redhat.com/show_bug.cgi?id=669549
about 'virsh setmem --config' not working for qemu.
Okay, I recognize it's a bug. So, about networks/disks, - qemudDomainAttachDeviceFlags() - qemuDomainUpdateDeviceFlags() - qemudDomainDetachDeviceFlags() ..should be modified to handle XML with --persistent option ?
2. virsh attach-disk doesn't seem to support misc. options. It doesn't have - boot_order - shareable - serial - io - error_policy etc...
If there's something that the libvirt API supports, but which virsh does not support, then that's a bug in virsh. Please let us know about these usability deficiencies in virsh, since that is the right place to be patching it for use by all other shell-based tools, rather than reinventing a new tool by every user.
We'll study what virsh does and start QEMU patches for these. Hmm...cmdAttachDisk() etc...seems just to make XML by printf()..okay, it seems not hard to modify virsh.c Thanks, -Kame

On 02/21/2011 07:20 PM, Eric Blake wrote:
On 02/21/2011 05:04 PM, KAMEZAWA Hiroyuki wrote:
On Mon, 21 Feb 2011 08:17:30 -0700 Eric Blake <eblake@redhat.com> wrote:
On 02/21/2011 01:23 AM, KAMEZAWA Hiroyuki wrote:
Hi, now, with qemu, virsh attach-disk doesn't work with inactive disks and we need to edit XML with virsh edit. IIUC, libvirt and virsh is designed as it is.
Actually, libvirt should be patched to learn how to modify xml of inactive disks for qemu (it already can do it for xen, so the API is already present, it's just that no one has wired up that API for qemu).
Before starging this, I thought of that. I did this in python by 3 reasons.
1. When we asked "Is it a spec that we cannot modify inactive domain ?" to a Redhat guy, he answered "it's a spec". Do you, maintainers, have some concensus about this ?
'virsh attach-disk --persistent' is supposed to be able to modify an inactive domain. If it doesn't do so for qemu, then that's because no one has yet implemented it correctly, which means libvirt has a bug that needs to be patched. For example, see:
https://bugzilla.redhat.com/show_bug.cgi?id=669549
about 'virsh setmem --config' not working for qemu.
Personally I don't think these flags should be implemented anywhere besides the xen driver where they are needed for back compat. Persistently adding devices is something that can be done by an API user in a few dozen lines of code, and it will work for all drivers. If we want virsh to expose this capability, we should just add a generic implementation there.
2. virsh attach-disk doesn't seem to support misc. options. It doesn't have - boot_order - shareable - serial - io - error_policy etc...
If there's something that the libvirt API supports, but which virsh does not support, then that's a bug in virsh. Please let us know about these usability deficiencies in virsh, since that is the right place to be patching it for use by all other shell-based tools, rather than reinventing a new tool by every user.
There are 2 distinct pieces of the libvirt API: API calls and XML building. Both are very large topics in their own right, and I think trying to have a single tool that exposes every nuance of both would be cumbersome. Right now virsh is mostly just an API call wrapper, which is what it should stay IMO. XML building/editing is also closely tied with validation, and higher level operations like creating storage or fixing storage permissions, none of which should live in virsh IMO. Thanks, Cole
participants (3)
-
Cole Robinson
-
Eric Blake
-
KAMEZAWA Hiroyuki