[PATCH] conf: fix block type CDROM cannot support startupPolicy
block type CDROM also support startupPolicy in the past, so let us fix it. Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f7025bffe4..dd374e8ab3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30853,9 +30853,10 @@ virDomainDiskTranslateSourcePool(virDomainDiskDef *def) } if (def->startupPolicy != 0 && - virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE) { + (virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE && + virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_BLOCK)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for 'file' type volume")); + _("'startupPolicy' is only valid for 'file' or 'block' type volume")); return -1; } -- 2.24.0.windows.2
On Tue, Sep 28, 2021 at 10:43 AM Jie Wang <wangjie88@huawei.com> wrote:
block type CDROM also support startupPolicy in the past, so
s/block/Block/ "in the past" could be more detailed. It's better if you tell from which version to which version the startupPolicy of block type cdrom is supported
let us fix it.
Signed-off-by: Jie Wang <wangjie88@huawei.com> --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f7025bffe4..dd374e8ab3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30853,9 +30853,10 @@ virDomainDiskTranslateSourcePool(virDomainDiskDef *def) }
if (def->startupPolicy != 0 && - virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE) { + (virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE && + virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_BLOCK)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for 'file' type volume")); + _("'startupPolicy' is only valid for 'file' or 'block' type volume")); return -1; }
-- 2.24.0.windows.2
On Thu, Sep 30, 2021 at 09:58:46 +0800, Han Han wrote:
On Tue, Sep 28, 2021 at 10:43 AM Jie Wang <wangjie88@huawei.com> wrote:
block type CDROM also support startupPolicy in the past, so
s/block/Block/
"in the past" could be more detailed. It's better if you tell from which version to which version the startupPolicy of block type cdrom is supported
Yes it indeed is, because it makes it easier for the reviewer. Specifically if it was working before it's a bit simpler since we don't need to check whether the code paths actually allow it. In addition the commit message mentions CDROM, but this is really changing it for all types of disks not only cdroms. So you need to update the commit message to be more clear what's happening. If this is fixng a regression, please attempt to find the commit that caused it. If not it's necessary to justify in the commit message that just removing the error is sufficient and that all code paths will be able to handle block devices just right. I can do that, but checking that everything works properly is more expensive for me, so it will take a while before I get back to this patch.
participants (3)
-
Han Han -
Jie Wang -
Peter Krempa