1:16 a.m.
As the description of removing CDROM media from
http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV
Add flag 'VSH_OFLAG_EMPTY_OK' to the option 'source' of attach-disk
Signed-off-by: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
---
tools/virsh.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 5544a41..e7eae74 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -11569,7 +11569,7 @@ static const vshCmdInfo info_attach_disk[] = {
static const vshCmdOptDef opts_attach_disk[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
- {"source", VSH_OT_DATA, VSH_OFLAG_REQ, N_("source of disk
device")},
+ {"source", VSH_OT_DATA, VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
N_("source of disk device")},
{"target", VSH_OT_DATA, VSH_OFLAG_REQ, N_("target of disk
device")},
{"driver", VSH_OT_STRING, 0, N_("driver of disk device")},
{"subdriver", VSH_OT_STRING, 0, N_("subdriver of disk device")},
--
1.7.5.4
5:08 p.m.
On 11/08/2011 12:16 AM, Xu He Jie wrote:
As the description of removing CDROM media from
http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV
Hmm.
virsh attach-disk --type cdrom --mode readonly myguest "" hdc
might look a bit nicer as:
virsh attach-disk --type cdrom --mode readonly myguest --target hdc
except that we marked --source as a required argument, so we have to
provide something even when there is no real source. So I agree that we
need your patch at a bare minimum to support this documented command
line for adding a cdrom drive without a disk.
Meanwhile, conf/domain_conf.c has this comment:
/* People sometimes pass a bogus '' source path
when they mean to omit the source element
completely. eg CDROM without media. This is
just a little compatability check to help
those broken apps */
if (source && STREQ(source, ""))
VIR_FREE(source);
So while passing an empty string down through the XML eventually does
the right thing, we make it sound like it is a gross hack, and that it
would be nicer if virsh weren't one of "those broken apps" that exploits
the xml hack.
Add flag 'VSH_OFLAG_EMPTY_OK' to the option 'source' of attach-disk
Signed-off-by: Xu He Jie<xuhj(a)linux.vnet.ibm.com>
---
tools/virsh.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 5544a41..e7eae74 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -11569,7 +11569,7 @@ static const vshCmdInfo info_attach_disk[] = {
static const vshCmdOptDef opts_attach_disk[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
- {"source", VSH_OT_DATA, VSH_OFLAG_REQ, N_("source of disk
device")},
+ {"source", VSH_OT_DATA, VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
N_("source of disk device")},
ACK to your change, but I'm reformatting it to fit 80 columns, then
squashing in this additional change before pushing so that virsh will be
a model citizen:
diff --git i/tools/virsh.c w/tools/virsh.c
index e7eae74..eed727b 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -11569,7 +11569,8 @@ static const vshCmdInfo info_attach_disk[] = {
static const vshCmdOptDef opts_attach_disk[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
- {"source", VSH_OT_DATA, VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
N_("source of disk device")},
+ {"source", VSH_OT_DATA, VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
+ N_("source of disk device")},
{"target", VSH_OT_DATA, VSH_OFLAG_REQ, N_("target of disk
device")},
{"driver", VSH_OT_STRING, 0, N_("driver of disk device")},
{"subdriver", VSH_OT_STRING, 0, N_("subdriver of disk
device")},
@@ -11754,6 +11755,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptString(cmd, "source", &source) <= 0)
goto cleanup;
+ /* Allow empty string as a placeholder that implies no source, for
+ * use in adding a cdrom drive with no disk. */
+ if (!*source)
+ source = NULL;
if (vshCommandOptString(cmd, "target", &target) <= 0)
goto cleanup;
@@ -11808,9 +11813,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (driver || subdriver || cache)
virBufferAddLit(&buf, "/>\n");
- virBufferAsprintf(&buf, " <source %s='%s'/>\n",
- (isFile) ? "file" : "dev",
- source);
+ if (source)
+ virBufferAsprintf(&buf, " <source %s='%s'/>\n",
+ (isFile) ? "file" : "dev",
+ source);
virBufferAsprintf(&buf, " <target dev='%s'/>\n",
target);
if (mode)
virBufferAsprintf(&buf, " <%s/>\n", mode);
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
8:34 p.m.
于 2011年11月09日 07:08, Eric Blake 写道:
On 11/08/2011 12:16 AM, Xu He Jie wrote:
> As the description of removing CDROM media from
> http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV
Hmm.
virsh attach-disk --type cdrom --mode readonly myguest "" hdc
might look a bit nicer as:
virsh attach-disk --type cdrom --mode readonly myguest --target hdc
except that we marked --source as a required argument, so we have to
provide something even when there is no real source. So I agree that
we need your patch at a bare minimum to support this documented
command line for adding a cdrom drive without a disk.
Is there any other method of
removing media from cdrom? I try with:
detach-disk myguest hdc
It told me: 'unsupported configuration: device type 'disk' cannot be
detached'
Meanwhile, conf/domain_conf.c has this comment:
/* People sometimes pass a bogus '' source path
when they mean to omit the source element
completely. eg CDROM without media. This is
just a little compatability check to help
those broken apps */
if (source && STREQ(source, ""))
VIR_FREE(source);
So while passing an empty string down through the XML eventually does
the right thing, we make it sound like it is a gross hack, and that it
would be nicer if virsh weren't one of "those broken apps" that
exploits the xml hack.
>
> Add flag 'VSH_OFLAG_EMPTY_OK' to the option 'source' of attach-disk
>
> Signed-off-by: Xu He Jie<xuhj(a)linux.vnet.ibm.com>
> ---
> tools/virsh.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 5544a41..e7eae74 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -11569,7 +11569,7 @@ static const vshCmdInfo info_attach_disk[] = {
>
> static const vshCmdOptDef opts_attach_disk[] = {
> {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
> - {"source", VSH_OT_DATA, VSH_OFLAG_REQ, N_("source of disk
device")},
> + {"source", VSH_OT_DATA, VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
> N_("source of disk device")},
ACK to your change, but I'm reformatting it to fit 80 columns, then
squashing in this additional change before pushing so that virsh will
be a model citizen:
Thanks!
diff --git i/tools/virsh.c w/tools/virsh.c
index e7eae74..eed727b 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -11569,7 +11569,8 @@ static const vshCmdInfo info_attach_disk[] = {
static const vshCmdOptDef opts_attach_disk[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
- {"source", VSH_OT_DATA, VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
N_("source of disk device")},
+ {"source", VSH_OT_DATA, VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
+ N_("source of disk device")},
{"target", VSH_OT_DATA, VSH_OFLAG_REQ, N_("target of disk device")},
{"driver", VSH_OT_STRING, 0, N_("driver of disk device")},
{"subdriver", VSH_OT_STRING, 0, N_("subdriver of disk device")},
@@ -11754,6 +11755,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd
*cmd)
if (vshCommandOptString(cmd, "source", &source) <= 0)
goto cleanup;
+ /* Allow empty string as a placeholder that implies no source, for
+ * use in adding a cdrom drive with no disk. */
+ if (!*source)
+ source = NULL;
if (vshCommandOptString(cmd, "target", &target) <= 0)
goto cleanup;
@@ -11808,9 +11813,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd
*cmd)
if (driver || subdriver || cache)
virBufferAddLit(&buf, "/>\n");
- virBufferAsprintf(&buf, " <source %s='%s'/>\n",
- (isFile) ? "file" : "dev",
- source);
+ if (source)
+ virBufferAsprintf(&buf, " <source %s='%s'/>\n",
+ (isFile) ? "file" : "dev",
+ source);
virBufferAsprintf(&buf, " <target dev='%s'/>\n", target);
if (mode)
virBufferAsprintf(&buf, " <%s/>\n", mode);
9:05 p.m.
On 11/09/2011 10:34 AM, Xu He Jie wrote:
于 2011年11月09日 07:08, Eric Blake 写道:
> On 11/08/2011 12:16 AM, Xu He Jie wrote:
>> As the description of removing CDROM media from
>> http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV
>
> Hmm.
> virsh attach-disk --type cdrom --mode readonly myguest "" hdc
> might look a bit nicer as:
> virsh attach-disk --type cdrom --mode readonly myguest --target hdc
> except that we marked --source as a required argument, so we have to
> provide something even when there is no real source. So I agree that
> we need your patch at a bare minimum to support this documented
> command line for adding a cdrom drive without a disk.
Is there any other method of removing media from cdrom? I try with:
detach-disk myguest hdc
This will detach the CD drive from guest, for just removing the media,
"update-device $domain xml" with the source path removed in the xml
will work.
We planned to add some new virsh commands such as "insert-media",
"eject-media" before to do the work, as "update-device" is not quite
visiable, and question like this are frequent in upstream, but
unfortunately,
it's not implemented yet.
It told me: 'unsupported configuration: device type
'disk' cannot be
detached'
Is this the exact error you saw, it's weried if a device of type disk
can't be
detached. The error expected should be something like:
"unsupported configuration: device type of 'cdrom' cannot be detached"
Regards
Osier
9:29 p.m.
于 2011年11月09日 11:05, Osier Yang 写道:
On 11/09/2011 10:34 AM, Xu He Jie wrote:
> 于 2011年11月09日 07:08, Eric Blake 写道:
>> On 11/08/2011 12:16 AM, Xu He Jie wrote:
>>> As the description of removing CDROM media from
>>> http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV
>>
>> Hmm.
>> virsh attach-disk --type cdrom --mode readonly myguest "" hdc
>> might look a bit nicer as:
>> virsh attach-disk --type cdrom --mode readonly myguest --target hdc
>> except that we marked --source as a required argument, so we have to
>> provide something even when there is no real source. So I agree that
>> we need your patch at a bare minimum to support this documented
>> command line for adding a cdrom drive without a disk.
> Is there any other method of removing media from cdrom? I try with:
> detach-disk myguest hdc
This will detach the CD drive from guest, for just removing the media,
"update-device $domain xml" with the source path removed in the xml
will work.
We planned to add some new virsh commands such as "insert-media",
"eject-media" before to do the work, as "update-device" is not quite
visiable, and question like this are frequent in upstream, but
unfortunately,
it's not implemented yet.
I understood, thanks.
> It told me: 'unsupported configuration: device type 'disk' cannot be
> detached'
Is this the exact error you saw, it's weried if a device of type disk
can't be
detached. The error expected should be something like:
"unsupported configuration: device type of 'cdrom' cannot be detached"
Yes, the xml as below:
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1'
unit='0'/>
</disk>
the command as below:
virsh # detach-disk ubuntu-test-vm1 hdc
错误： Failed to detach disk
错误： unsupported configuration: device type 'disk' cannot be detached
Regards
Osier
9:57 p.m.
On 11/09/2011 11:29 AM, Xu He Jie wrote:
于 2011年11月09日 11:05, Osier Yang 写道:
> On 11/09/2011 10:34 AM, Xu He Jie wrote:
>> 于 2011年11月09日 07:08, Eric Blake 写道:
>>> On 11/08/2011 12:16 AM, Xu He Jie wrote:
>>>> As the description of removing CDROM media from
>>>> http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV
>>>
>>> Hmm.
>>> virsh attach-disk --type cdrom --mode readonly myguest "" hdc
>>> might look a bit nicer as:
>>> virsh attach-disk --type cdrom --mode readonly myguest --target hdc
>>> except that we marked --source as a required argument, so we have
>>> to provide something even when there is no real source. So I agree
>>> that we need your patch at a bare minimum to support this
>>> documented command line for adding a cdrom drive without a disk.
>> Is there any other method of removing media from cdrom? I try with:
>> detach-disk myguest hdc
>
> This will detach the CD drive from guest, for just removing the media,
> "update-device $domain xml" with the source path removed in the xml
> will work.
>
> We planned to add some new virsh commands such as "insert-media",
> "eject-media" before to do the work, as "update-device" is not
quite
> visiable, and question like this are frequent in upstream, but
> unfortunately,
> it's not implemented yet.
I understood, thanks.
>
>> It told me: 'unsupported configuration: device type 'disk' cannot be
>> detached'
>
> Is this the exact error you saw, it's weried if a device of type disk
> can't be
> detached. The error expected should be something like:
>
> "unsupported configuration: device type of 'cdrom' cannot be
detached"
>
Yes, the xml as below:
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1'
unit='0'/>
</disk>
the command as below:
virsh # detach-disk ubuntu-test-vm1 hdc
错误： Failed to detach disk
错误： unsupported configuration: device type 'disk' cannot be detached
I see, there is a bug here. This patch fixed it.
http://www.redhat.com/archives/libvir-list/2011-November/msg00355.html
Osier
4762
days inactive
4763
days old
5 comments
3 participants
participants (3)
-
Eric Blake -
Osier Yang -
Xu He Jie