[Libvir] [RFC] Device attach/detach on virsh
I want to add I/F to do attach/detatch of VIF and VBD to virsh with virDomainAttachDevice()/virDomainDetachDevice(). And, I have two proposals about I/F for virsh to do attach/detach of VIF and VBD. proposal 1: Virsh catches MAC, bridge name, device name (physical,virtual), and another by the command option. ex) ------------------------------------------------------------------ # virsh help attach(detach)-vif(vbd) NAME attach(detach)-vif(vbd) - attach(detach) vif(vbd) SYNOPSIS * VIF attach(detach)-vif <domain> <MAC> <bridge> ... or * VBD attach(detach)-vbd <domain> <virt-dev> ... DESCRIPTION Attach(Detach) vif(vbd) device OPTIONS <domain> domain name, id or uuid * VIF <MAC> MAC address of vif <bridge> bridge name of vif ... * VBD <virt-dev> virtual device name of vbd <phy-dev> physical device name of vbd ... ------------------------------------------------------------------ <advantage> - I/F is easy to use than proposal 1. (Because the user does not have to make XML) <disadvantage> - I/F increases because I/F of VIF and VBD becomes separate. (So, the addition of I/F is necessary at any time for supporting devices other than VIF and VBD. ) - Handling of optional analysis, handling of XML making are necessary in virsh.c, and processing becomes complicated. proposal 2: virsh catches a definition of a device by XML ex) ------------------------------------------------------------------ # virsh help attach(detach)-device NAME attach(detach)-device - attach(detach) device from an XML file SYNOPSIS attach(detach)-device <domain> <file> DESCRIPTION Attach(Detach) device from an XML <file> OPTIONS <domain> domain name, id or uuid <file> XML file of device description ------------------------------------------------------------------ <advantage> - I/F is unified without affecting a device type. (I/F is simple) - Handling of virsh.c is simple. (Optional analysis is not necessary) <disadvantage> - The user has to describe XML.(It is troublesome) I think that specifications that a user is easy to use (proposal 1) are better. Please, give me an opinion which proposal is better. Thanks, Masayuki Sunou
On Thu, May 10, 2007 at 06:50:53PM +0900, Masayuki Sunou wrote:
I want to add I/F to do attach/detatch of VIF and VBD to virsh with virDomainAttachDevice()/virDomainDetachDevice(). And, I have two proposals about I/F for virsh to do attach/detach of VIF and VBD.
proposal 1: Virsh catches MAC, bridge name, device name (physical,virtual), and another by the command option. [...] <advantage> - I/F is easy to use than proposal 1. (Because the user does not have to make XML) <disadvantage> - I/F increases because I/F of VIF and VBD becomes separate. (So, the addition of I/F is necessary at any time for supporting devices other than VIF and VBD. ) - Handling of optional analysis, handling of XML making are necessary in virsh.c, and processing becomes complicated.
To me this proposal is not okay as-is because it looks completely tied to Xen. But maybe I didn't understand, suppose I use KVM what would be the vbd or vif parameter looking like ? We need at least to change the terminology i.e. replace vif and vbd terms, but I'm afraid One important problem is naming, suppose you want to remove a network device, how will you name that device ? Using a vif Xen device number is not proper in my opinion it makes it really hard for the user (i.e. you have to dig in Xen internal to find the number).
proposal 2: virsh catches a definition of a device by XML
ex) ------------------------------------------------------------------ # virsh help attach(detach)-device NAME attach(detach)-device - attach(detach) device from an XML file
SYNOPSIS attach(detach)-device <domain> <file>
DESCRIPTION Attach(Detach) device from an XML <file>
OPTIONS <domain> domain name, id or uuid <file> XML file of device description ------------------------------------------------------------------
<advantage> - I/F is unified without affecting a device type. (I/F is simple) - Handling of virsh.c is simple. (Optional analysis is not necessary) <disadvantage> - The user has to describe XML.(It is troublesome)
I think that specifications that a user is easy to use (proposal 1) are better. Please, give me an opinion which proposal is better.
it looks to me that only proposal 2 is not tied to a given engine and would work even if we add very different system with more complex devices. But I agree it's not perfect from a user point of view either. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Thu, May 10, 2007 at 12:50:40PM -0400, Daniel Veillard wrote:
On Thu, May 10, 2007 at 06:50:53PM +0900, Masayuki Sunou wrote:
I want to add I/F to do attach/detatch of VIF and VBD to virsh with virDomainAttachDevice()/virDomainDetachDevice(). And, I have two proposals about I/F for virsh to do attach/detach of VIF and VBD.
proposal 1: Virsh catches MAC, bridge name, device name (physical,virtual), and another by the command option. [...] <advantage> - I/F is easy to use than proposal 1. (Because the user does not have to make XML) <disadvantage> - I/F increases because I/F of VIF and VBD becomes separate. (So, the addition of I/F is necessary at any time for supporting devices other than VIF and VBD. ) - Handling of optional analysis, handling of XML making are necessary in virsh.c, and processing becomes complicated.
To me this proposal is not okay as-is because it looks completely tied to Xen. But maybe I didn't understand, suppose I use KVM what would be the vbd or vif parameter looking like ? We need at least to change the terminology i.e. replace vif and vbd terms, but I'm afraid
Huh ? I didn't see anything in this proposal which was Xen-specific. The disks where being identified based on their backend path (eg /var/lib/xen/image/foo.img or /dev/sda4), while network cards were being identified based on their MAC address. Both of those are unique identifiers used by pretty much any virt system.
One important problem is naming, suppose you want to remove a network device, how will you name that device ? Using a vif Xen device number is not proper in my opinion it makes it really hard for the user (i.e. you have to dig in Xen internal to find the number).
MAC address.
proposal 2: virsh catches a definition of a device by XML
ex) ------------------------------------------------------------------ # virsh help attach(detach)-device NAME attach(detach)-device - attach(detach) device from an XML file
SYNOPSIS attach(detach)-device <domain> <file>
DESCRIPTION Attach(Detach) device from an XML <file>
OPTIONS <domain> domain name, id or uuid <file> XML file of device description ------------------------------------------------------------------
<advantage> - I/F is unified without affecting a device type. (I/F is simple) - Handling of virsh.c is simple. (Optional analysis is not necessary) <disadvantage> - The user has to describe XML.(It is troublesome)
I think that specifications that a user is easy to use (proposal 1) are better. Please, give me an opinion which proposal is better.
it looks to me that only proposal 2 is not tied to a given engine and would work even if we add very different system with more complex devices. But I agree it's not perfect from a user point of view either.
Yeah, its utterly horrible for end users to use, but at the same time could be useful for automation / tools. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Fri, May 11, 2007 at 12:21:00AM +0100, Daniel P. Berrange wrote:
On Thu, May 10, 2007 at 12:50:40PM -0400, Daniel Veillard wrote:
On Thu, May 10, 2007 at 06:50:53PM +0900, Masayuki Sunou wrote: To me this proposal is not okay as-is because it looks completely tied to Xen. But maybe I didn't understand, suppose I use KVM what would be the vbd or vif parameter looking like ? We need at least to change the terminology i.e. replace vif and vbd terms, but I'm afraid
Huh ? I didn't see anything in this proposal which was Xen-specific. The
Hum, sorry I misunderstood, I though it was using the vif and vbd internal Xen numbers to adress the device. I was focusing on the delete operation, and wondering what was the naming used.
disks where being identified based on their backend path (eg /var/lib/xen/image/foo.img or /dev/sda4), while network cards were being identified based on their MAC address. Both of those are unique identifiers used by pretty much any virt system.
yup objection removed, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Hi I understood as follows. * There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion? And, I am going to correct about naming. vif --> interface vbd --> disk Thanks, Masayuki Sunou In message <20070511053125.GA30832@redhat.com> "Re: [Libvir] [RFC] Device attach/detach on virsh" "Daniel Veillard <veillard@redhat.com>" wrote:
On Fri, May 11, 2007 at 12:21:00AM +0100, Daniel P. Berrange wrote:
On Thu, May 10, 2007 at 12:50:40PM -0400, Daniel Veillard wrote:
On Thu, May 10, 2007 at 06:50:53PM +0900, Masayuki Sunou wrote: To me this proposal is not okay as-is because it looks completely tied to Xen. But maybe I didn't understand, suppose I use KVM what would be the vbd or vif parameter looking like ? We need at least to change the terminology i.e. replace vif and vbd terms, but I'm afraid
Huh ? I didn't see anything in this proposal which was Xen-specific. The
Hum, sorry I misunderstood, I though it was using the vif and vbd internal Xen numbers to adress the device. I was focusing on the delete operation, and wondering what was the naming used.
disks where being identified based on their backend path (eg /var/lib/xen/image/foo.img or /dev/sda4), while network cards were being identified based on their MAC address. Both of those are unique identifiers used by pretty much any virt system.
yup objection removed,
Daniel
-- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Fri, May 11, 2007 at 06:42:10PM +0900, Masayuki Sunou wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
+1 Gets my vote/ Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Fri, May 11, 2007 at 06:42:10PM +0900, Masayuki Sunou wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
And, I am going to correct about naming. vif --> interface vbd --> disk
That would be excellent ! Jet-lag and lack of sleep made me confuse the vif and vbd in the name with actual arguments. Changing the names is fine, maybe we can get better words: maybe add-netif/remove-netif instead of just using 'interface' which is very generic, and maybe using add-block/remove-block instead of disk, some devices may actually not be disk (e.g. tape drive), but it's just a minor suggestion. thanks a lot ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Fri, May 11, 2007 at 02:16:51PM -0400, Daniel Veillard wrote:
On Fri, May 11, 2007 at 06:42:10PM +0900, Masayuki Sunou wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
And, I am going to correct about naming. vif --> interface vbd --> disk
That would be excellent ! Jet-lag and lack of sleep made me confuse the vif and vbd in the name with actual arguments. Changing the names is fine, maybe we can get better words: maybe add-netif/remove-netif instead of just using 'interface' which is very generic, and maybe using add-block/remove-block instead of disk, some devices may actually not be disk (e.g. tape drive), but it's just a minor suggestion.
I disagree. We alread use 'interface' and 'disk' in the XML description of devices, so its better to keep consistent terminology IMHO. tap, floppy, cdrom, harddisk are all just sub-types of disk - which we deal with by using the 'device' attribute in the <disk> element. Regards, Dan -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Fri, May 11, 2007 at 07:19:39PM +0100, Daniel P. Berrange wrote:
On Fri, May 11, 2007 at 02:16:51PM -0400, Daniel Veillard wrote: I disagree. We alread use 'interface' and 'disk' in the XML description of devices, so its better to keep consistent terminology IMHO. tap, floppy,
okay :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Hi This patch adds virsh attach-device/detach-device. Currently it supports a definition of a device by XML(proposal 2). I plan to submit command option version (proposal 1) later. Any comments are welcome! Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. -------------------------------------------------------------------------------- Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.75 diff -u -p -r1.75 virsh.c --- src/virsh.c 3 May 2007 16:03:02 -0000 1.75 +++ src/virsh.c 16 May 2007 00:56:21 -0000 @@ -2403,6 +2403,139 @@ cmdVNCDisplay(vshControl * ctl, vshCmd * return ret; } +/* + * "attach-device" command + */ +static vshCmdInfo info_attach_device[] = { + {"syntax", "attach-device <domain> <file> "}, + {"help", gettext_noop("attach device from an XML file")}, + {"desc", gettext_noop("Attach device from an XML <file>.")}, + {NULL, NULL} +}; + +static vshCmdOptDef opts_attach_device[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {"file", VSH_OT_DATA, 0, gettext_noop("XML file")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdAttachDevice(vshControl * ctl, vshCmd * cmd) +{ + virDomainPtr dom; + char *from; + char buffer[BUFSIZ]; + int ret = TRUE; + int found; + int fd, l; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", NULL))) + return FALSE; + + from = vshCommandOptString(cmd, "file", &found); + if (!found) { + virDomainFree(dom); + return FALSE; + } + + fd = open(from, O_RDONLY); + if (fd < 0) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + virDomainFree(dom); + return FALSE; + } + l = read(fd, &buffer[0], sizeof(buffer)); + if ((l <= 0) || (l >= (int) sizeof(buffer))) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + buffer[l] = 0; + + ret = virDomainAttachDevice(dom, &buffer[0]); + if (ret < 0) { + vshError(ctl, FALSE, _("Failed to attach device from %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + + close(fd); + virDomainFree(dom); + return TRUE; +} + + +/* + * "detach-device" command + */ +static vshCmdInfo info_detach_device[] = { + {"syntax", "detach-device <domain> <file> "}, + {"help", gettext_noop("detach device from an XML file")}, + {"desc", gettext_noop("Detach device from an XML <file>")}, + {NULL, NULL} +}; + +static vshCmdOptDef opts_detach_device[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {"file", VSH_OT_DATA, 0, gettext_noop("XML file")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdDetachDevice(vshControl * ctl, vshCmd * cmd) +{ + virDomainPtr dom; + char *from; + char buffer[BUFSIZ]; + int ret = TRUE; + int found; + int fd, l; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", NULL))) + return FALSE; + + from = vshCommandOptString(cmd, "file", &found); + if (!found) { + virDomainFree(dom); + return FALSE; + } + + fd = open(from, O_RDONLY); + if (fd < 0) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + virDomainFree(dom); + return FALSE; + } + l = read(fd, &buffer[0], sizeof(buffer)); + if ((l <= 0) || (l >= (int) sizeof(buffer))) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + buffer[l] = 0; + + ret = virDomainDetachDevice(dom, &buffer[0]); + if (ret < 0) { + vshError(ctl, FALSE, _("Failed to detach device from %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + + close(fd); + virDomainFree(dom); + return TRUE; +} + /* * "quit" command @@ -2467,6 +2600,8 @@ static vshCmdDef commands[] = { {"vcpupin", cmdVcpupin, opts_vcpupin, info_vcpupin}, {"version", cmdVersion, NULL, info_version}, {"vncdisplay", cmdVNCDisplay, opts_vncdisplay, info_vncdisplay}, + {"attach-device", cmdAttachDevice, opts_attach_device, info_attach_device}, + {"detach-device", cmdDetachDevice, opts_detach_device, info_detach_device}, {NULL, NULL, NULL, NULL} }; -------------------------------------------------------------------------------- In message <200705111842.IJH95853.739E2KNG@aa.jp.fujitsu.com> "Re: [Libvir] [RFC] Device attach/detach on virsh" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
And, I am going to correct about naming. vif --> interface vbd --> disk
Hi Would you give me a comment on this? Thanks, Masayuki Sunou In message <200705161354.DEI30254.NE73GK92@aa.jp.fujitsu.com> "[Libvir] [PATCH] Device attach/detach on virsh(XML version)" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote: Hi This patch adds virsh attach-device/detach-device. Currently it supports a definition of a device by XML(proposal 2). I plan to submit command option version (proposal 1) later. Any comments are welcome! Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. -------------------------------------------------------------------------------- Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.75 diff -u -p -r1.75 virsh.c --- src/virsh.c 3 May 2007 16:03:02 -0000 1.75 +++ src/virsh.c 16 May 2007 00:56:21 -0000 @@ -2403,6 +2403,139 @@ cmdVNCDisplay(vshControl * ctl, vshCmd * return ret; } +/* + * "attach-device" command + */ +static vshCmdInfo info_attach_device[] = { + {"syntax", "attach-device <domain> <file> "}, + {"help", gettext_noop("attach device from an XML file")}, + {"desc", gettext_noop("Attach device from an XML <file>.")}, + {NULL, NULL} +}; + +static vshCmdOptDef opts_attach_device[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {"file", VSH_OT_DATA, 0, gettext_noop("XML file")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdAttachDevice(vshControl * ctl, vshCmd * cmd) +{ + virDomainPtr dom; + char *from; + char buffer[BUFSIZ]; + int ret = TRUE; + int found; + int fd, l; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", NULL))) + return FALSE; + + from = vshCommandOptString(cmd, "file", &found); + if (!found) { + virDomainFree(dom); + return FALSE; + } + + fd = open(from, O_RDONLY); + if (fd < 0) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + virDomainFree(dom); + return FALSE; + } + l = read(fd, &buffer[0], sizeof(buffer)); + if ((l <= 0) || (l >= (int) sizeof(buffer))) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + buffer[l] = 0; + + ret = virDomainAttachDevice(dom, &buffer[0]); + if (ret < 0) { + vshError(ctl, FALSE, _("Failed to attach device from %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + + close(fd); + virDomainFree(dom); + return TRUE; +} + + +/* + * "detach-device" command + */ +static vshCmdInfo info_detach_device[] = { + {"syntax", "detach-device <domain> <file> "}, + {"help", gettext_noop("detach device from an XML file")}, + {"desc", gettext_noop("Detach device from an XML <file>")}, + {NULL, NULL} +}; + +static vshCmdOptDef opts_detach_device[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {"file", VSH_OT_DATA, 0, gettext_noop("XML file")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdDetachDevice(vshControl * ctl, vshCmd * cmd) +{ + virDomainPtr dom; + char *from; + char buffer[BUFSIZ]; + int ret = TRUE; + int found; + int fd, l; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", NULL))) + return FALSE; + + from = vshCommandOptString(cmd, "file", &found); + if (!found) { + virDomainFree(dom); + return FALSE; + } + + fd = open(from, O_RDONLY); + if (fd < 0) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + virDomainFree(dom); + return FALSE; + } + l = read(fd, &buffer[0], sizeof(buffer)); + if ((l <= 0) || (l >= (int) sizeof(buffer))) { + vshError(ctl, FALSE, _("Failed to read description file %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + buffer[l] = 0; + + ret = virDomainDetachDevice(dom, &buffer[0]); + if (ret < 0) { + vshError(ctl, FALSE, _("Failed to detach device from %s"), from); + close(fd); + virDomainFree(dom); + return FALSE; + } + + close(fd); + virDomainFree(dom); + return TRUE; +} + /* * "quit" command @@ -2467,6 +2600,8 @@ static vshCmdDef commands[] = { {"vcpupin", cmdVcpupin, opts_vcpupin, info_vcpupin}, {"version", cmdVersion, NULL, info_version}, {"vncdisplay", cmdVNCDisplay, opts_vncdisplay, info_vncdisplay}, + {"attach-device", cmdAttachDevice, opts_attach_device, info_attach_device}, + {"detach-device", cmdDetachDevice, opts_detach_device, info_detach_device}, {NULL, NULL, NULL, NULL} }; -------------------------------------------------------------------------------- In message <200705111842.IJH95853.739E2KNG@aa.jp.fujitsu.com> "Re: [Libvir] [RFC] Device attach/detach on virsh" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
And, I am going to correct about naming. vif --> interface vbd --> disk
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Masayuki Sunou wrote:
Hi
Would you give me a comment on this?
I'm working on a new version of this patch. Back shortly ... Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
I tidied up the patch and some surrounding code, and came up with the attached patch. Unfortunately I don't have a means to test this, but if someone can test attaching and detaching networks and devices, then I have no objection to it going in. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Richard W.M. Jones wrote:
+ buffer = realloc (buffer, len+1); + if (buffer == NULL) goto out_of_memory; + buffer[len] = '\0'; + return buffer;
Euch, add 'close (fd);' in there ... Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Hi Rich I apply this patch, and test virsh attach-device/detach-device. As a result, libvirt has no problem about attaching and detaching disk/insterface. But, because option flag of <file> in virsh attach-device/detach-device is wrong, I correct it.
Euch, add 'close (fd);' in there ...
And, 'close (fd);' is added to this patch. Thanks, Masayuki Sunou In message <4652FB40.3060700@redhat.com> "Re: [Libvir] [PATCH] Device attach/detach on virsh(XML version)" ""Richard W.M. Jones" <rjones@redhat.com>" wrote:
Richard W.M. Jones wrote:
+ buffer = realloc (buffer, len+1); + if (buffer == NULL) goto out_of_memory; + buffer[len] = '\0'; + return buffer;
Euch, add 'close (fd);' in there ...
Rich.
-- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
------------------------------------------------------------------------------- Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.75 diff -u -p -r1.75 virsh.c --- src/virsh.c 3 May 2007 16:03:02 -0000 1.75 +++ src/virsh.c 23 May 2007 05:04:56 -0000 @@ -22,6 +22,7 @@ #include <string.h> #include <stdarg.h> #include <unistd.h> +#include <errno.h> #include <getopt.h> #include <sys/types.h> #include <sys/time.h> @@ -673,6 +674,49 @@ static vshCmdOptDef opts_create[] = { {NULL, 0, 0, NULL} }; +/* Read in a whole file and return it as a string. + * If it fails, it logs an error and returns NULL. + * String must be freed by caller. + */ +static char * +readFile (vshControl *ctl, const char *filename) +{ + char *buffer = NULL, *oldbuffer; + int len = 0, fd, r; + char b[1024]; + + fd = open (filename, O_RDONLY); + if (fd == -1) { + file_error: + vshError (ctl, FALSE, "%s: %s", filename, strerror (errno)); + error: + if (buffer) free (buffer); + if (fd >= 0) close (fd); + return NULL; + } + + for (;;) { + r = read (fd, b, sizeof b); + if (r == -1) goto file_error; + if (r == 0) break; /* End of file. */ + oldbuffer = buffer; + buffer = realloc (buffer, len+r); + if (buffer == NULL) { + out_of_memory: + vshError (ctl, FALSE, "realloc: %s", strerror (errno)); + goto error; + } + memcpy (buffer+len, b, r); + len += r; + } + + buffer = realloc (buffer, len+1); + if (buffer == NULL) goto out_of_memory; + buffer[len] = '\0'; + close (fd); + return buffer; +} + static int cmdCreate(vshControl * ctl, vshCmd * cmd) { @@ -680,8 +724,7 @@ cmdCreate(vshControl * ctl, vshCmd * cmd char *from; int found; int ret = TRUE; - char buffer[BUFSIZ]; - int fd, l; + char *buffer; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -690,19 +733,12 @@ cmdCreate(vshControl * ctl, vshCmd * cmd if (!found) return FALSE; - fd = open(from, O_RDONLY); - if (fd < 0) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - return(FALSE); - } - l = read(fd, &buffer[0], sizeof(buffer)); - if ((l <= 0) || (l >= (int) sizeof(buffer))) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - close(fd); - return(FALSE); - } - buffer[l] = 0; - dom = virDomainCreateLinux(ctl->conn, &buffer[0], 0); + buffer = readFile (ctl, from); + if (buffer == NULL) return FALSE; + + dom = virDomainCreateLinux(ctl->conn, buffer, 0); + free (buffer); + if (dom != NULL) { vshPrint(ctl, _("Domain %s created from %s\n"), virDomainGetName(dom), from); @@ -735,8 +771,7 @@ cmdDefine(vshControl * ctl, vshCmd * cmd char *from; int found; int ret = TRUE; - char buffer[BUFSIZ]; - int fd, l; + char *buffer; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -745,19 +780,12 @@ cmdDefine(vshControl * ctl, vshCmd * cmd if (!found) return FALSE; - fd = open(from, O_RDONLY); - if (fd < 0) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - return(FALSE); - } - l = read(fd, &buffer[0], sizeof(buffer)); - if ((l <= 0) || (l >= (int) sizeof(buffer))) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - close(fd); - return(FALSE); - } - buffer[l] = 0; - dom = virDomainDefineXML(ctl->conn, &buffer[0]); + buffer = readFile (ctl, from); + if (buffer == NULL) return FALSE; + + dom = virDomainDefineXML(ctl->conn, buffer); + free (buffer); + if (dom != NULL) { vshPrint(ctl, _("Domain %s defined from %s\n"), virDomainGetName(dom), from); @@ -1800,8 +1828,7 @@ cmdNetworkCreate(vshControl * ctl, vshCm char *from; int found; int ret = TRUE; - char buffer[BUFSIZ]; - int fd, l; + char *buffer; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1810,19 +1837,12 @@ cmdNetworkCreate(vshControl * ctl, vshCm if (!found) return FALSE; - fd = open(from, O_RDONLY); - if (fd < 0) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - return(FALSE); - } - l = read(fd, &buffer[0], sizeof(buffer)); - if ((l <= 0) || (l >= (int) sizeof(buffer))) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - close(fd); - return(FALSE); - } - buffer[l] = 0; - network = virNetworkCreateXML(ctl->conn, &buffer[0]); + buffer = readFile (ctl, from); + if (buffer == NULL) return FALSE; + + network = virNetworkCreateXML(ctl->conn, buffer); + free (buffer); + if (network != NULL) { vshPrint(ctl, _("Network %s created from %s\n"), virNetworkGetName(network), from); @@ -1856,8 +1876,7 @@ cmdNetworkDefine(vshControl * ctl, vshCm char *from; int found; int ret = TRUE; - char buffer[BUFSIZ]; - int fd, l; + char *buffer; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1866,19 +1885,12 @@ cmdNetworkDefine(vshControl * ctl, vshCm if (!found) return FALSE; - fd = open(from, O_RDONLY); - if (fd < 0) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - return(FALSE); - } - l = read(fd, &buffer[0], sizeof(buffer)); - if ((l <= 0) || (l >= (int) sizeof(buffer))) { - vshError(ctl, FALSE, _("Failed to read description file %s"), from); - close(fd); - return(FALSE); - } - buffer[l] = 0; - network = virNetworkDefineXML(ctl->conn, &buffer[0]); + buffer = readFile (ctl, from); + if (buffer == NULL) return FALSE; + + network = virNetworkDefineXML(ctl->conn, buffer); + free (buffer); + if (network != NULL) { vshPrint(ctl, _("Network %s defined from %s\n"), virNetworkGetName(network), from); @@ -2403,6 +2415,113 @@ cmdVNCDisplay(vshControl * ctl, vshCmd * return ret; } +/* + * "attach-device" command + */ +static vshCmdInfo info_attach_device[] = { + {"syntax", "attach-device <domain> <file> "}, + {"help", gettext_noop("attach device from an XML file")}, + {"desc", gettext_noop("Attach device from an XML <file>.")}, + {NULL, NULL} +}; + +static vshCmdOptDef opts_attach_device[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("XML file")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdAttachDevice(vshControl * ctl, vshCmd * cmd) +{ + virDomainPtr dom; + char *from; + char *buffer; + int ret = TRUE; + int found; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", NULL))) + return FALSE; + + from = vshCommandOptString(cmd, "file", &found); + if (!found) { + virDomainFree(dom); + return FALSE; + } + + buffer = readFile (ctl, from); + if (buffer == NULL) return FALSE; + + ret = virDomainAttachDevice(dom, buffer); + free (buffer); + + if (ret < 0) { + vshError(ctl, FALSE, _("Failed to attach device from %s"), from); + virDomainFree(dom); + return FALSE; + } + + virDomainFree(dom); + return TRUE; +} + + +/* + * "detach-device" command + */ +static vshCmdInfo info_detach_device[] = { + {"syntax", "detach-device <domain> <file> "}, + {"help", gettext_noop("detach device from an XML file")}, + {"desc", gettext_noop("Detach device from an XML <file>")}, + {NULL, NULL} +}; + +static vshCmdOptDef opts_detach_device[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("XML file")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdDetachDevice(vshControl * ctl, vshCmd * cmd) +{ + virDomainPtr dom; + char *from; + char *buffer; + int ret = TRUE; + int found; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", NULL))) + return FALSE; + + from = vshCommandOptString(cmd, "file", &found); + if (!found) { + virDomainFree(dom); + return FALSE; + } + + buffer = readFile (ctl, from); + if (buffer == NULL) return FALSE; + + ret = virDomainDetachDevice(dom, buffer); + free (buffer); + + if (ret < 0) { + vshError(ctl, FALSE, _("Failed to detach device from %s"), from); + virDomainFree(dom); + return FALSE; + } + + virDomainFree(dom); + return TRUE; +} + /* * "quit" command @@ -2467,6 +2586,8 @@ static vshCmdDef commands[] = { {"vcpupin", cmdVcpupin, opts_vcpupin, info_vcpupin}, {"version", cmdVersion, NULL, info_version}, {"vncdisplay", cmdVNCDisplay, opts_vncdisplay, info_vncdisplay}, + {"attach-device", cmdAttachDevice, opts_attach_device, info_attach_device}, + {"detach-device", cmdDetachDevice, opts_detach_device, info_detach_device}, {NULL, NULL, NULL, NULL} }; -------------------------------------------------------------------------------
On Tue, 2007-05-22 at 15:16 +0100, Richard W.M. Jones wrote:
Richard W.M. Jones wrote:
+ buffer = realloc (buffer, len+1); + if (buffer == NULL) goto out_of_memory;
Note, if realloc() fails, the original buffer isn't freed. So, you want to make sure you free the original one. Cheers, Mark.
Mark McLoughlin wrote:
On Tue, 2007-05-22 at 15:16 +0100, Richard W.M. Jones wrote:
Richard W.M. Jones wrote:
+ buffer = realloc (buffer, len+1); + if (buffer == NULL) goto out_of_memory;
Note, if realloc() fails, the original buffer isn't freed. So, you want to make sure you free the original one.
I even kept the oldbuffer around for this reason, but then forgot to free it! Grrrr ... garbage.collection++ Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
OK, here we go with an updated patch. I think this should go in. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Hi, I also agree garbage collection. Anyway, this kind of problem should be checked by valgrind. Thanks Atsushi SAKAI "Richard W.M. Jones" <rjones@redhat.com> wrote:
Mark McLoughlin wrote:
On Tue, 2007-05-22 at 15:16 +0100, Richard W.M. Jones wrote:
Richard W.M. Jones wrote:
+ buffer = realloc (buffer, len+1); + if (buffer == NULL) goto out_of_memory;
Note, if realloc() fails, the original buffer isn't freed. So, you want to make sure you free the original one.
I even kept the oldbuffer around for this reason, but then forgot to free it!
Grrrr ... garbage.collection++
Rich.
-- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Wed, 2007-05-23 at 17:52 +0900, Atsushi SAKAI wrote:
Hi,
I also agree garbage collection. Anyway, this kind of problem should be checked by valgrind.
The only way to test code paths like this is to set up a test harness which will re-run the code over and over, with a malloc() implementation that will fail at random points. Cheers, Mark.
Hi Rich, On Tue, 2007-05-22 at 15:10 +0100, Richard W.M. Jones wrote:
+static char * +readFile (vshControl *ctl, const char *filename) +{ + char *buffer = NULL, *oldbuffer; + int len = 0, fd, r; + char b[1024]; + + fd = open (filename, O_RDONLY); + if (fd == -1) { + file_error: + vshError (ctl, FALSE, "%s: %s", filename, strerror (errno)); + error: + if (buffer) free (buffer); + if (fd >= 0) close (fd); + return NULL; + } + + for (;;) { + r = read (fd, b, sizeof b); + if (r == -1) goto file_error; + if (r == 0) break; /* End of file. */ + oldbuffer = buffer; + buffer = realloc (buffer, len+r); + if (buffer == NULL) { + out_of_memory: + vshError (ctl, FALSE, "realloc: %s", strerror (errno)); + goto error; + } + memcpy (buffer+len, b, r); + len += r; + } + + buffer = realloc (buffer, len+1); + if (buffer == NULL) goto out_of_memory; + buffer[len] = '\0'; + return buffer; +}
Truly, the way you're using gotos here gives me a serious headache. Following the logic of the function involves jumping back and forth around the code way too much. I noticed you using that style in the remote patch too. I'd suggest something more like: static char * readFile(vshControl *ctl, const char *filename) { char *retval; int len = 0, fd; if ((fd = open(filename, O_RDONLY)) < 0) { vshError (ctl, FALSE, "Failed to open '%s': %s", filename, strerror (errno)); return NULL; } if (!(retval = malloc(len + 1))) goto out_of_memory; while (1) { char buffer[1024]; char *new; int ret; if ((ret = read(fd, buffer, sizeof(buffer))) == 0) break; if (ret == -1) { if (errno == EINTR) continue; vshError (ctl, FALSE, "Failed to open '%s': %s", filename, strerror (errno)); goto error; } if (!(new = realloc(retval, len + ret + 1))) goto out_of_memory; retval = new; memcpy(retval + len, buffer, ret); len += ret; } retval[len] = '\0'; return buffer; out_of_memory: vshError (ctl, FALSE, "Error allocating memory: %s", strerror(errno)); error: if (retval) free(retval); close(fd); return NULL; } You can read this version straight through without having to jump back to an earlier part of the function. Cheers, Mark.
OK, here we go ... Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Masayuki: I have applied this patch to CVS. Thanks for your contribution to libvirt. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Hi This patch adds virsh attach-device/detach-device. This supports a definition of a device by command option(proposal 1). I examine the following opinions, and "attach-interface" supports only two common case (bridge, virtual network).
Networking is more complicated because there are 8 (!) different ways to connected up VMs so far. We could either have huge numbers of args, or we could just support the two common cases (bridge, virtual network) and say for more complex cases use full XML.
Any comments are welcome! Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. In message <200705111842.IJH95853.739E2KNG@aa.jp.fujitsu.com> "Re: [Libvir] [RFC] Device attach/detach on virsh" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
And, I am going to correct about naming. vif --> interface vbd --> disk
Hi Would you give me a comment on this patch? Thanks, Masayuki Sunou. In message <200705291541.BEI87583.29KNGE37@aa.jp.fujitsu.com> "[Libvir] [PATCH] Device attach/detach on virsh (command optionversion)" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote: Hi This patch adds virsh attach-device/detach-device. This supports a definition of a device by command option(proposal 1). I examine the following opinions, and "attach-interface" supports only two common case (bridge, virtual network).
Networking is more complicated because there are 8 (!) different ways to connected up VMs so far. We could either have huge numbers of args, or we could just support the two common cases (bridge, virtual network) and say for more complex cases use full XML.
Any comments are welcome! Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. In message <200705111842.IJH95853.739E2KNG@aa.jp.fujitsu.com> "Re: [Libvir] [RFC] Device attach/detach on virsh" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
And, I am going to correct about naming. vif --> interface vbd --> disk
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi Would you give me a comment on this patch? Thanks, Masayuki Sunou. In message <200705291541.BEI87583.29KNGE37@aa.jp.fujitsu.com> "[Libvir] [PATCH] Device attach/detach on virsh (command optionversion)" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote:
Hi
This patch adds virsh attach-device/detach-device. This supports a definition of a device by command option(proposal 1).
I examine the following opinions, and "attach-interface" supports only two common case (bridge, virtual network).
Networking is more complicated because there are 8 (!) different ways to connected up VMs so far. We could either have huge numbers of args, or we could just support the two common cases (bridge, virtual network) and say for more complex cases use full XML.
Any comments are welcome!
Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>
Thanks, Masayuki Sunou.
In message <200705111842.IJH95853.739E2KNG@aa.jp.fujitsu.com> "Re: [Libvir] [RFC] Device attach/detach on virsh" "Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>" wrote:
Hi
I understood as follows.
* There is no problem in a proposal 1 and a proposal 2 * It is better that the user can choose use of XML and use of command-option by the situation
Therefore, I want to add both proposal 1 and proposal 2 to virsh. There is no problem in this opinion?
And, I am going to correct about naming. vif --> interface vbd --> disk
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jun 19, 2007 at 05:38:58PM +0900, Masayuki Sunou wrote:
Hi
Would you give me a comment on this patch?
oh right, I knew I had forgotten it for 0.2.3 but didn't yet look at it again. Stylistically I would have preferred if creating the XML was done using the xmlBuffer APIs, but it's cosmetic, so I applied it it's in CVS, thanks a lot and sorry for the delay, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Thu, May 10, 2007 at 06:50:53PM +0900, Masayuki Sunou wrote:
I want to add I/F to do attach/detatch of VIF and VBD to virsh with virDomainAttachDevice()/virDomainDetachDevice(). And, I have two proposals about I/F for virsh to do attach/detach of VIF and VBD.
proposal 1: Virsh catches MAC, bridge name, device name (physical,virtual), and another by the command option.
ex) ------------------------------------------------------------------ # virsh help attach(detach)-vif(vbd) NAME attach(detach)-vif(vbd) - attach(detach) vif(vbd)
SYNOPSIS * VIF attach(detach)-vif <domain> <MAC> <bridge> ... or * VBD attach(detach)-vbd <domain> <virt-dev> ...
I'd probably call them attach-disk attach-interface to match the name used within the <devices> block so we don't have lots of different terminolgy for the same concept.
DESCRIPTION Attach(Detach) vif(vbd) device
OPTIONS <domain> domain name, id or uuid
* VIF <MAC> MAC address of vif <bridge> bridge name of vif ...
* VBD <virt-dev> virtual device name of vbd <phy-dev> physical device name of vbd ... ------------------------------------------------------------------
<advantage> - I/F is easy to use than proposal 1. (Because the user does not have to make XML) <disadvantage> - I/F increases because I/F of VIF and VBD becomes separate. (So, the addition of I/F is necessary at any time for supporting devices other than VIF and VBD. )
I think that's fine.
- Handling of optional analysis, handling of XML making are necessary in virsh.c, and processing becomes complicated.
For attaching device's there'll be a variety of optional flags you'll need to support. eg to specify the driver & subtype, file, phys, tap:aio, tap:qcow, etc' to specify the front end type (floppy, disk, cdrom). Some real world examples virsh attach-disk /dev/sda1 xvdc virsh attach-disk --driver phy /dev/sda1 xvdc virsh attach-disk --type cdrom --driver tap --subdriver aio /root/boot.iso hdc virsh attach-disk --driver file /var/lib/xen/data.img xvdd Networking is more complicated because there are 8 (!) different ways to connected up VMs so far. We could either have huge numbers of args, or we could just support the two common cases (bridge, virtual network) and say for more complex cases use full XML. virsh attach-interface bridge eth0 virsh attach-interface --mac 02:23:44:e2:2 network default virsh attach-interface --target vnet3 network default
proposal 2: virsh catches a definition of a device by XML
ex) ------------------------------------------------------------------ # virsh help attach(detach)-device NAME attach(detach)-device - attach(detach) device from an XML file
SYNOPSIS attach(detach)-device <domain> <file>
DESCRIPTION Attach(Detach) device from an XML <file>
OPTIONS <domain> domain name, id or uuid <file> XML file of device description ------------------------------------------------------------------
<advantage> - I/F is unified without affecting a device type. (I/F is simple) - Handling of virsh.c is simple. (Optional analysis is not necessary) <disadvantage> - The user has to describe XML.(It is troublesome)
I don't think we should consider this an either/or option - lets do both. ie, have specific attach-disk/interface for convenience, and have the generic attach-device for broad support. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (6)
-
Atsushi SAKAI -
Daniel P. Berrange -
Daniel Veillard -
Mark McLoughlin -
Masayuki Sunou -
Richard W.M. Jones