[PATCH 1/8] Added a few attach-disk parameters

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 76 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 8 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12b35c037d..16227085cc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -224,6 +224,26 @@ static const vshCmdOptDef opts_attach_disk[] = { .flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK, .help = N_("source of disk device") }, + {.name = "source-protocol", + .type = VSH_OT_STRING, + .help = N_("protocol used by disk device source") + } + {.name = "source-name", + .type = VSH_OT_STRING, + .help = N_("name of disk device source") + }, + {.name = "source-host-name", + .type = VSH_OT_STRING, + .help = N_("host name for source of disk device") + }, + {.name = "source-host-transport", + .type = VSH_OT_STRING, + .help = N_("host transport for source of disk device") + }, + {.name = "source-host-socket", + .type = VSH_OT_STRING, + .help = N_("host socket for source of disk device") + }, {.name = "target", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -562,11 +582,13 @@ static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - const char *source = NULL, *target = NULL, *driver = NULL, - *subdriver = NULL, *type = NULL, *mode = NULL, - *iothread = NULL, *cache = NULL, *io = NULL, - *serial = NULL, *straddr = NULL, *wwn = NULL, - *targetbus = NULL, *alias = NULL; + const char *source = NULL, *source_name = NULL, *source_protocol = NULL, + *target = NULL, *driver = NULL, *subdriver = NULL, *type = NULL, + *mode = NULL, *iothread = NULL, *cache = NULL, + *io = NULL, *serial = NULL, *straddr = NULL, + *wwn = NULL, *targetbus = NULL, *alias = NULL, + *host_name = NULL, *host_transport = NULL, + *host_port = NULL, *host_socket = NULL; struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; @@ -591,6 +613,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_AFFECT_LIVE; if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-name", &source_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-protocol", &source_protocol) < 0 || vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 || vshCommandOptStringReq(ctl, cmd, "subdriver", &subdriver) < 0 || @@ -604,7 +628,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 || vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 || vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || - vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0) + vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-name", &host_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-transport", &host_transport) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-socket", &host_socket) < 0) goto cleanup; if (!stype) { @@ -659,9 +686,42 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferAddLit(&buf, "/>\n"); } - if (source) - virBufferAsprintf(&buf, "<source %s='%s'/>\n", + if (source || source_protocol || source_name || + host_name || host_transport || host_socket) { + virBufferAsprintf(&buf, "<source"); + + if (source) + virBufferAsprintf(&buf, " %s='%s'", isFile ? "file" : "dev", source); + if (source_protocol) + virBufferAsprintf(&buf, " protocol='%s'", source_protocol); + if (source_name) + virBufferAsprintf(&buf, " name='%s'", source_name); + + if (host_name || host_transport || host_socket) { + virBufferAsprintf(">\n<host"); + + if (host_name) { + host_port = strchr(host_name, ':'); + + if (!host_port) + virBufferAsprintf(" name='%s'", host_name); + else { + host_name[host_port - host_name] = '\0'; + virBufferAsprintf(" name='%s' port='%s'", host_name, host_port + 1); + } + } + if (host_transport) + virBufferAsprintf(" transport='%s'", host_transport); + if (host_socket) + virBufferAsprintf(" socket='%s'", host_socket); + + virBufferAsprintf(" />\n</source>\n"); + } else { + virBufferAsprintf(" />\n"); + } + } + virBufferAsprintf(&buf, "<target dev='%s'", target); if (targetbus) virBufferAsprintf(&buf, " bus='%s'", targetbus); -- 2.29.0

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 16227085cc..810e55fa53 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -707,7 +707,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (!host_port) virBufferAsprintf(" name='%s'", host_name); else { - host_name[host_port - host_name] = '\0'; + host_name[(int)(host_port - host_name)] = '\0'; virBufferAsprintf(" name='%s' port='%s'", host_name, host_port + 1); } } -- 2.29.0

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 810e55fa53..5862993464 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -688,7 +688,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (source || source_protocol || source_name || host_name || host_transport || host_socket) { - virBufferAsprintf(&buf, "<source"); + virBufferAddLit(&buf, "<source"); if (source) virBufferAsprintf(&buf, " %s='%s'", @@ -699,26 +699,27 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferAsprintf(&buf, " name='%s'", source_name); if (host_name || host_transport || host_socket) { - virBufferAsprintf(">\n<host"); + virBufferAddLit(&buf, ">\n<host"); if (host_name) { host_port = strchr(host_name, ':'); - if (!host_port) - virBufferAsprintf(" name='%s'", host_name); - else { + if (!host_port) { + virBufferAsprintf(&buf, " name='%s'", host_name); + } else { host_name[(int)(host_port - host_name)] = '\0'; - virBufferAsprintf(" name='%s' port='%s'", host_name, host_port + 1); + virBufferAsprintf(&buf, " name='%s' port='%s'", host_name, host_port + 1); } } + if (host_transport) - virBufferAsprintf(" transport='%s'", host_transport); + virBufferAsprintf(&buf, " transport='%s'", host_transport); if (host_socket) - virBufferAsprintf(" socket='%s'", host_socket); + virBufferAsprintf(&buf, " socket='%s'", host_socket); - virBufferAsprintf(" />\n</source>\n"); + virBufferAddLit(&buf, " />\n</source>\n"); } else { - virBufferAsprintf(" />\n"); + virBufferAddLit(&buf, " />\n"); } } -- 2.29.0

On Tue, Nov 10, 2020 at 15:56:15 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 810e55fa53..5862993464 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -688,7 +688,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
if (source || source_protocol || source_name || host_name || host_transport || host_socket) { - virBufferAsprintf(&buf, "<source"); + virBufferAddLit(&buf, "<source");
This is fixing code you've added in patch 1. Please squash all fixups together. There's no point to commit broken code and fix it later.

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 5862993464..cda0531a37 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -587,8 +587,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) *mode = NULL, *iothread = NULL, *cache = NULL, *io = NULL, *serial = NULL, *straddr = NULL, *wwn = NULL, *targetbus = NULL, *alias = NULL, - *host_name = NULL, *host_transport = NULL, - *host_port = NULL, *host_socket = NULL; + *host_transport = NULL, *host_port = NULL, *host_socket = NULL; struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; @@ -596,6 +595,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *stype = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *xml = NULL; + char *host_name = NULL; struct stat st; bool current = vshCommandOptBool(cmd, "current"); bool config = vshCommandOptBool(cmd, "config"); -- 2.29.0

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cda0531a37..9d00cb8b21 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -227,7 +227,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {.name = "source-protocol", .type = VSH_OT_STRING, .help = N_("protocol used by disk device source") - } + }, {.name = "source-name", .type = VSH_OT_STRING, .help = N_("name of disk device source") @@ -629,7 +629,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 || vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0 || - vshCommandOptStringReq(ctl, cmd, "source-host-name", &host_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-name", (const char **) &host_name) < 0 || vshCommandOptStringReq(ctl, cmd, "source-host-transport", &host_transport) < 0 || vshCommandOptStringReq(ctl, cmd, "source-host-socket", &host_socket) < 0) goto cleanup; -- 2.29.0

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9d00cb8b21..ae9a2b1101 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -711,7 +711,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferAsprintf(&buf, " name='%s' port='%s'", host_name, host_port + 1); } } - if (host_transport) virBufferAsprintf(&buf, " transport='%s'", host_transport); if (host_socket) -- 2.29.0

On Tue, Nov 10, 2020 at 15:56:18 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 1 - 1 file changed, 1 deletion(-)
This definitely does not belong to a final submission.

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 70 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ae9a2b1101..64c7c454bd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -578,6 +578,49 @@ static int str2DiskAddress(const char *str, struct DiskAddress *diskAddr) return -1; } +static void attachDiskHostGen(virBufferPtr buf, const vshCmd *cmd) +{ + // Can be multiple hosts so we have to scan + // the cmd options to find all the host params + // <source tag in XML not yet closed + vshCmdOpt *candidate = cmd->opts; + char *host_name = NULL, *host_port = NULL; + int closeTag = 0; + + while (candidate) { + // Iterate candidates to find each host-name + if (STREQ(candidate->def->name, "source-host-name")) { + // After the first host-name, we need to terminate + // the <host ... tag + // It's left open so socket and transport can be added later + if (closeTag) + virBufferAddLit(buf, "/>\n"); + else + closeTag = 1; + + host_name = candidate->data; + host_port = strchr(host_name, ':'); + + if (!host_port) { + // If port isn't provided, only print name + virBufferAsprintf(buf, "<host name='%s'", host_name); + } else { + // If port is provided, manipulate strings and print both + host_name[(int)(host_port - host_name)] = '\0'; + virBufferAsprintf(buf, "<host name='%s' port='%s'", host_name, host_port + 1); + } + } else if (STREQ(candidate->def->name, "source-host-socket")) { + virBufferAsprintf(buf, " socket='%s'", candidate->data); + } else if (STREQ(candidate->def->name, "source-host-transport")) { + virBufferAsprintf(buf, " transport='%s'", candidate->data); + } + + candidate = candidate->next; + } + // Close final <host tag + virBufferAddLit(buf, "/>\n"); +} + static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { @@ -587,7 +630,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) *mode = NULL, *iothread = NULL, *cache = NULL, *io = NULL, *serial = NULL, *straddr = NULL, *wwn = NULL, *targetbus = NULL, *alias = NULL, - *host_transport = NULL, *host_port = NULL, *host_socket = NULL; + *host_transport = NULL, *host_name = NULL, *host_socket = NULL; struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; @@ -595,7 +638,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *stype = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; char *xml = NULL; - char *host_name = NULL; struct stat st; bool current = vshCommandOptBool(cmd, "current"); bool config = vshCommandOptBool(cmd, "config"); @@ -698,27 +740,25 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (source_name) virBufferAsprintf(&buf, " name='%s'", source_name); - if (host_name || host_transport || host_socket) { + if (!(host_name || host_transport || host_socket)) { + // Close source if no host is provided + virBufferAddLit(&buf, "/>\n"); + } else if (!host_name) { + // If no host name is provided but there is a host, + // we have a single host with params virBufferAddLit(&buf, ">\n<host"); - if (host_name) { - host_port = strchr(host_name, ':'); - - if (!host_port) { - virBufferAsprintf(&buf, " name='%s'", host_name); - } else { - host_name[(int)(host_port - host_name)] = '\0'; - virBufferAsprintf(&buf, " name='%s' port='%s'", host_name, host_port + 1); - } - } if (host_transport) virBufferAsprintf(&buf, " transport='%s'", host_transport); if (host_socket) virBufferAsprintf(&buf, " socket='%s'", host_socket); - virBufferAddLit(&buf, " />\n</source>\n"); + virBufferAddLit(&buf, "/>\n</source>\n"); } else { - virBufferAddLit(&buf, " />\n"); + // May have multiple hosts, use helper method + virBufferAddLit(&buf, ">\n"); + attachDiskHostGen(&buf, cmd); + virBufferAddLit(&buf, "</source>\n"); } } -- 2.29.0

Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- docs/manpages/virsh.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index bfd26e3120..83134ba571 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4500,9 +4500,11 @@ attach-disk [--current]] | [--persistent]] [--targetbus bus] [--driver driver] [--subdriver subdriver] [--iothread iothread] [--cache cache] [--io io] [--type type] [--alias alias] - [--mode mode] [--sourcetype sourcetype] [--serial serial] - [--wwn wwn] [--rawio] [--address address] [--multifunction] - [--print-xml] + [--mode mode] [--sourcetype sourcetype] [--source-name name] + [--source-protocol protocol] [--source-host-name hostname:port] + [--source-host-transport transport] [--source-host-socket socket] + [--serial serial] [--wwn wwn] [--rawio] [--address address] + [--multifunction] [--print-xml] Attach a new disk device to the domain. *source* is path for the files and devices. *target* controls the bus or -- 2.29.0

On Tue, Nov 10, 2020 at 15:56:20 -0600, Ryan Gahagan wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- docs/manpages/virsh.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index bfd26e3120..83134ba571 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4500,9 +4500,11 @@ attach-disk [--current]] | [--persistent]] [--targetbus bus] [--driver driver] [--subdriver subdriver] [--iothread iothread] [--cache cache] [--io io] [--type type] [--alias alias] - [--mode mode] [--sourcetype sourcetype] [--serial serial] - [--wwn wwn] [--rawio] [--address address] [--multifunction] - [--print-xml] + [--mode mode] [--sourcetype sourcetype] [--source-name name] + [--source-protocol protocol] [--source-host-name hostname:port] + [--source-host-transport transport] [--source-host-socket socket] + [--serial serial] [--wwn wwn] [--rawio] [--address address] + [--multifunction] [--print-xml]
You also need to describe how the parameters work in the text below.

It is better to format the patch summary like this format: "SUBSYSTEM: TITLE" For example, this patch could be "virsh: Added a few attach-disk parameters" You can see the git log of libvirt for more reference: https://libvirt.org/git/?p=libvirt.git;a=summary On Wed, Nov 11, 2020 at 5:58 AM Ryan Gahagan <rgahagan@cs.utexas.edu> wrote:
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 76 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 8 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12b35c037d..16227085cc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -224,6 +224,26 @@ static const vshCmdOptDef opts_attach_disk[] = { .flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK, .help = N_("source of disk device") }, + {.name = "source-protocol", + .type = VSH_OT_STRING, + .help = N_("protocol used by disk device source") + } + {.name = "source-name", + .type = VSH_OT_STRING, + .help = N_("name of disk device source") + }, + {.name = "source-host-name", + .type = VSH_OT_STRING, + .help = N_("host name for source of disk device") + }, + {.name = "source-host-transport", + .type = VSH_OT_STRING, + .help = N_("host transport for source of disk device") + }, + {.name = "source-host-socket", + .type = VSH_OT_STRING, + .help = N_("host socket for source of disk device") + }, {.name = "target", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -562,11 +582,13 @@ static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - const char *source = NULL, *target = NULL, *driver = NULL, - *subdriver = NULL, *type = NULL, *mode = NULL, - *iothread = NULL, *cache = NULL, *io = NULL, - *serial = NULL, *straddr = NULL, *wwn = NULL, - *targetbus = NULL, *alias = NULL; + const char *source = NULL, *source_name = NULL, *source_protocol = NULL, + *target = NULL, *driver = NULL, *subdriver = NULL, *type = NULL, + *mode = NULL, *iothread = NULL, *cache = NULL, + *io = NULL, *serial = NULL, *straddr = NULL, + *wwn = NULL, *targetbus = NULL, *alias = NULL, + *host_name = NULL, *host_transport = NULL, + *host_port = NULL, *host_socket = NULL; struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; @@ -591,6 +613,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_AFFECT_LIVE;
if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-name", &source_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-protocol", &source_protocol) < 0 || vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 || vshCommandOptStringReq(ctl, cmd, "subdriver", &subdriver) < 0 || @@ -604,7 +628,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 || vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 || vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || - vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0) + vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-name", &host_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-transport", &host_transport) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-socket", &host_socket) < 0) goto cleanup;
if (!stype) { @@ -659,9 +686,42 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferAddLit(&buf, "/>\n"); }
- if (source) - virBufferAsprintf(&buf, "<source %s='%s'/>\n", + if (source || source_protocol || source_name || + host_name || host_transport || host_socket) { + virBufferAsprintf(&buf, "<source"); + + if (source) + virBufferAsprintf(&buf, " %s='%s'", isFile ? "file" : "dev", source); + if (source_protocol) + virBufferAsprintf(&buf, " protocol='%s'", source_protocol); + if (source_name) + virBufferAsprintf(&buf, " name='%s'", source_name); + + if (host_name || host_transport || host_socket) { + virBufferAsprintf(">\n<host"); + + if (host_name) { + host_port = strchr(host_name, ':'); + + if (!host_port) + virBufferAsprintf(" name='%s'", host_name); + else { + host_name[host_port - host_name] = '\0'; + virBufferAsprintf(" name='%s' port='%s'", host_name, host_port + 1); + } + } + if (host_transport) + virBufferAsprintf(" transport='%s'", host_transport); + if (host_socket) + virBufferAsprintf(" socket='%s'", host_socket); + + virBufferAsprintf(" />\n</source>\n"); + } else { + virBufferAsprintf(" />\n"); + } + } + virBufferAsprintf(&buf, "<target dev='%s'", target); if (targetbus) virBufferAsprintf(&buf, " bus='%s'", targetbus); -- 2.29.0

On Tue, Nov 10, 2020 at 15:56:13 -0600, Ryan Gahagan wrote: Please describe your changes in the commit message.
Signed-off-by: Ryan Gahagan <rgahagan@cs.utexas.edu> --- tools/virsh-domain.c | 76 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 8 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12b35c037d..16227085cc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -224,6 +224,26 @@ static const vshCmdOptDef opts_attach_disk[] = { .flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK, .help = N_("source of disk device") }, + {.name = "source-protocol", + .type = VSH_OT_STRING, + .help = N_("protocol used by disk device source") + } + {.name = "source-name", + .type = VSH_OT_STRING, + .help = N_("name of disk device source") + }, + {.name = "source-host-name", + .type = VSH_OT_STRING, + .help = N_("host name for source of disk device") + }, + {.name = "source-host-transport", + .type = VSH_OT_STRING, + .help = N_("host transport for source of disk device") + }, + {.name = "source-host-socket", + .type = VSH_OT_STRING, + .help = N_("host socket for source of disk device") + }, {.name = "target", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -562,11 +582,13 @@ static bool cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - const char *source = NULL, *target = NULL, *driver = NULL, - *subdriver = NULL, *type = NULL, *mode = NULL, - *iothread = NULL, *cache = NULL, *io = NULL, - *serial = NULL, *straddr = NULL, *wwn = NULL, - *targetbus = NULL, *alias = NULL; + const char *source = NULL, *source_name = NULL, *source_protocol = NULL, + *target = NULL, *driver = NULL, *subdriver = NULL, *type = NULL, + *mode = NULL, *iothread = NULL, *cache = NULL, + *io = NULL, *serial = NULL, *straddr = NULL, + *wwn = NULL, *targetbus = NULL, *alias = NULL, + *host_name = NULL, *host_transport = NULL, + *host_port = NULL, *host_socket = NULL;
We prefer one declaration per line with explicit type. Prior art here is wrong, but there's no need to fix it. Just add your variables on separate lines.
struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; @@ -591,6 +613,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_AFFECT_LIVE;
if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-name", &source_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-protocol", &source_protocol) < 0 || vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 || vshCommandOptStringReq(ctl, cmd, "subdriver", &subdriver) < 0 || @@ -604,7 +628,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 || vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 || vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 || - vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0) + vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-name", &host_name) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-transport", &host_transport) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-host-socket", &host_socket) < 0) goto cleanup;
if (!stype) { @@ -659,9 +686,42 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferAddLit(&buf, "/>\n"); }
- if (source) - virBufferAsprintf(&buf, "<source %s='%s'/>\n", + if (source || source_protocol || source_name || + host_name || host_transport || host_socket) { + virBufferAsprintf(&buf, "<source"); + + if (source) + virBufferAsprintf(&buf, " %s='%s'", isFile ? "file" : "dev", source); + if (source_protocol) + virBufferAsprintf(&buf, " protocol='%s'", source_protocol); + if (source_name) + virBufferAsprintf(&buf, " name='%s'", source_name);
So, --source-name is mutually exclusive with --source. Please record this using VSH_EXCLUSIVE_OPTIONS_VAR as we do for other arguments. --source-name also requires --source-protocol, which also should be recorded.
+ + if (host_name || host_transport || host_socket) { + virBufferAsprintf(">\n<host"); + + if (host_name) { + host_port = strchr(host_name, ':'); + + if (!host_port) + virBufferAsprintf(" name='%s'", host_name); + else { + host_name[host_port - host_name] = '\0';
You must not modify pointers you get from vshCommandOptStringReq. Copy the string if you need to modify it.
+ virBufferAsprintf(" name='%s' port='%s'", host_name, host_port + 1); + } + } + if (host_transport) + virBufferAsprintf(" transport='%s'", host_transport); + if (host_socket) + virBufferAsprintf(" socket='%s'", host_socket); + + virBufferAsprintf(" />\n</source>\n");
Note that the result of the above will be an ugly formatted XML, but we strive for it to look correct. You'll need to use virBufferAdjustIndent and use only one newline per formatting API since only the last newline in a formating API use (virBufferAsprintf and such) triggers automatic indentation.
+ } else { + virBufferAsprintf(" />\n"); + } + } + virBufferAsprintf(&buf, "<target dev='%s'", target); if (targetbus) virBufferAsprintf(&buf, " bus='%s'", targetbus); -- 2.29.0
participants (3)
-
Han Han
-
Peter Krempa
-
Ryan Gahagan