qemu_monitor: set optional args on demand to qmp migrate

[luckyh]

The blk and the inc options were always set to the qmp migrate messagei, disrespecting the defaults set by qemu. However, the two options were removed since qemu 9.1.

This patch changes the building of the arguments for the new usage.

ID: 2596

[luckyh]

Hi @aliang123 @YongxueHong would you mind to help review this patch? thanks in advance!

[YongxueHong]

There is a bit of a typo in the comment message: The blk and the inc options were always set to the qmp migrate messagei -> message

[aliang123]

Test pass for the qmp_monitor part: (1/1) Host_RHEL.m9.u5.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.blockdev_inc_backup_with_migration.q35: PASS (167.92 s)

But have something need to confirm here:

hmp monitor

def migrate(self, uri, full_copy=False, incremental_copy=False, wait=False): -> The default value for blk and inc is False ..... cmd = "migrate" if not wait: cmd += " -d" if full_copy: cmd += " -b" if incremental_copy: cmd += " -i" cmd += " %s" % uri return self.cmd(cmd)

qmp monitor

def migrate(self, uri, full_copy=None, incremental_copy=None, wait=False): -> The default for blk and inc is None ..... args = {"uri": uri} if full_copy is not None: args["blk"] = full_copy if incremental_copy is not None: args["inc"] = incremental_copy args["uri"] = re.sub('"', "", args["uri"]) try: return self.cmd("migrate", args) except QMPCmdError as e: if e.data["class"] in ["SockConnectInprogress", "GenericError"]: LOG.debug("Migrate socket connection still initializing...") else: raise e

• Should we keep the same default value for def migrate() in both qmp and hmp?
• wait=False is defined in def migrate() in qmp, but it is not used at all. What we should do on it? Should we report a new bug to track it or just let it be? I know it's out of scope of this patch.

[luckyh]

Does this new usage also work on the hmp?

Should we keep the same default value for def migrate() in both qmp and hmp?

@YongxueHong @aliang123 the reason I didn't touch the hmp method was that using the current code had already met our requirement. But it's true that it would make readers understand the code easier by unifying the default values. So let me adopt your suggestion, thanks!

[luckyh]

wait=False is defined in def migrate() in qmp, but it is not used at all. What we should do on it? Should we report a new bug to track it or just let it be? I know it's out of scope of this patch.

@aliang123 good catch! I guess the previous committer's intent was to keep the same declaration/prototype for the two methods. Feel free to file an issue for tracking that, thanks!

[aliang123]

Acked