Host assisted clone should adjust requested storage when cloning from block to filesystem volume mode (#3900) #3901

gaohoward · 2025-09-10T13:39:27Z

When cloning from a source, the host cloner should check the requested size for the target tmp pvc if the tmp source pvc is of block volume mode and the target pvc is of filesystem volume mode because it should take filesystem overhead into consideration and enlarge the original request size accordingly
before creating the tmp pvc.

Because currently the check is missing the cloning will fail on validation.

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

https://issues.redhat.com/browse/CNV-44140

Special notes for your reviewer:

Release note:

Fix: Host assisted clone should adjust requested storage when cloning from block to filesystem volume mode

kubevirt-bot · 2025-09-10T13:39:36Z

Hi @gaohoward. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

kubevirt-bot · 2025-09-10T13:39:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign aglitke for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

akalenyu · 2025-09-11T07:38:03Z

/test all

coveralls · 2025-09-11T07:49:30Z

coverage: 49.428% (+0.04%) from 49.388%
when pulling b606f62 on gaohoward:cnv-44140
into c5480cc on kubevirt:main.

awels · 2025-10-13T13:38:11Z

I am probably missing something, but didn't #3384 fix this particular issue? Or did I miss a particular case with that PR?

gaohoward · 2025-10-13T14:27:10Z

I am probably missing something, but didn't #3384 fix this particular issue? Or did I miss a particular case with that PR?

Yeah I think it goes a different path. In this case it falls back to host-clone from smart-clone (supported by provisioner), and in that case it didn't take the fs overhead into account.

awels · 2025-10-13T14:55:39Z

Gotcha, okay, let me know if you need a review on the PR.

gaohoward · 2025-10-14T08:18:48Z

Gotcha, okay, let me know if you need a review on the PR.

Yes pleas review. Thanks @awels !

pkg/controller/clone/host-clone.go

gaohoward · 2025-10-23T09:00:22Z

/retest

gaohoward · 2025-10-23T14:23:37Z

/retest

gaohoward · 2025-10-27T01:11:00Z

/retest

pkg/controller/clone/host-clone.go

gaohoward · 2025-11-06T15:11:16Z

/retest

gaohoward · 2025-12-02T07:53:37Z

/retest

gaohoward · 2025-12-03T03:37:40Z

/retest

pkg/controller/datavolume/pvc-clone-controller.go

pkg/controller/common/util.go

pkg/controller/clone/host-clone.go

pkg/controller/common/util.go

gaohoward · 2025-12-08T14:51:58Z

/retest

akalenyu

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.

I am a little confused that we have to inflate in the populator layer;
usually we do this in the mutating pvc webhook (on by default)/dv controllers

gaohoward · 2025-12-11T02:24:03Z

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.

I am a little confused that we have to inflate in the populator layer; usually we do this in the mutating pvc webhook (on by default)/dv controllers

Here you are:
https://issues.redhat.com/browse/CNV-44140

Yes the issue happens at the very late stage where the snapshot clone falls back to host assisted clone.

akalenyu · 2025-12-11T17:08:55Z

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.
I am a little confused that we have to inflate in the populator layer; usually we do this in the mutating pvc webhook (on by default)/dv controllers

Here you are: https://issues.redhat.com/browse/CNV-44140

Yes the issue happens at the very late stage where the snapshot clone falls back to host assisted clone.

I see, though I am still confused by inflating this far into the process. Doesn't the target PVC get inflated anyhow by the webhook/DV controller (and thus so does the temp)?

akalenyu · 2025-12-11T17:32:24Z

Also note that bug describes a separate issue that may still exist

$ oc create  -f dv-hpp-cloned-from-datasource.yaml
Error from server: error when creating "dv-hpp-cloned-from-datasource.yaml": admission webhook "datavolume-validate.cdi.kubevirt.io" denied the request:  Storage size is missing

akalenyu · 2025-12-11T17:33:24Z

/cc @jpeimer

kubevirt-bot · 2025-12-11T17:33:26Z

@akalenyu: GitHub didn't allow me to request PR reviews from the following users: jpeimer.

Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @jpeimer

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

gaohoward · 2025-12-12T10:33:59Z

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.
I am a little confused that we have to inflate in the populator layer; usually we do this in the mutating pvc webhook (on by default)/dv controllers

Here you are: https://issues.redhat.com/browse/CNV-44140
Yes the issue happens at the very late stage where the snapshot clone falls back to host assisted clone.

I see, though I am still confused by inflating this far into the process. Doesn't the target PVC get inflated anyhow by the webhook/DV controller (and thus so does the temp)?

No the target PVC doesn't get inflated when doing snapshot clone. When the controller plans the clone it finds out the storage class is different from the volume snapshot's one and then it fall back to host assisted clone. When doing host clone phase it create the tmp target PVC, if we don't check here then it will report the failure because the target size is not enough (for this to happen the tmp target PVC volume mode must be filesystem).

gaohoward · 2025-12-12T10:34:54Z

dv-hpp-cloned-from-datasource.yaml

It doesn't happen for me on upstream. Can you share your yaml file?

akalenyu · 2025-12-16T13:46:38Z

dv-hpp-cloned-from-datasource.yaml

It doesn't happen for me on upstream. Can you share your yaml file?

Yeah I don't see it either, may have to do with an older version

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.
I am a little confused that we have to inflate in the populator layer; usually we do this in the mutating pvc webhook (on by default)/dv controllers

Here you are: https://issues.redhat.com/browse/CNV-44140
Yes the issue happens at the very late stage where the snapshot clone falls back to host assisted clone.

I see, though I am still confused by inflating this far into the process. Doesn't the target PVC get inflated anyhow by the webhook/DV controller (and thus so does the temp)?

No the target PVC doesn't get inflated when doing snapshot clone. When the controller plans the clone it finds out the storage class is different from the volume snapshot's one and then it fall back to host assisted clone. When doing host clone phase it create the tmp target PVC, if we don't check here then it will report the failure because the target size is not enough (for this to happen the tmp target PVC volume mode must be filesystem).

I was pretty sure we inflate filesystem target PVCs here -

containerized-data-importer/pkg/controller/datavolume/util.go

Line 314 in e147bbd

    
           func renderPvcSpecVolumeSize(client client.Client, pvcSpec *v1.PersistentVolumeClaimSpec, isClone bool, log *logr.Logger) error {

Sorry for poking I just think we have a problem elsewhere and that inflating the temp PVC in the populator will just cover that up

gaohoward · 2025-12-16T14:56:46Z

dv-hpp-cloned-from-datasource.yaml

It doesn't happen for me on upstream. Can you share your yaml file?

Yeah I don't see it either, may have to do with an older version

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.
I am a little confused that we have to inflate in the populator layer; usually we do this in the mutating pvc webhook (on by default)/dv controllers

Here you are: https://issues.redhat.com/browse/CNV-44140
Yes the issue happens at the very late stage where the snapshot clone falls back to host assisted clone.

I see, though I am still confused by inflating this far into the process. Doesn't the target PVC get inflated anyhow by the webhook/DV controller (and thus so does the temp)?

No the target PVC doesn't get inflated when doing snapshot clone. When the controller plans the clone it finds out the storage class is different from the volume snapshot's one and then it fall back to host assisted clone. When doing host clone phase it create the tmp target PVC, if we don't check here then it will report the failure because the target size is not enough (for this to happen the tmp target PVC volume mode must be filesystem).

I was pretty sure we inflate filesystem target PVCs here -

containerized-data-importer/pkg/controller/datavolume/util.go

Line 314 in e147bbd

func renderPvcSpecVolumeSize(client client.Client, pvcSpec *v1.PersistentVolumeClaimSpec, isClone bool, log *logr.Logger) error {

Sorry for poking I just think we have a problem elsewhere and that inflating the target in the populator will just cover that up

This method is never called in this case. Actually the webhook rendering is not enabled. For snapshot cloning it basically fallback to host-assisted during the planning phase, and when the host-clone phase is handling the temp target pvc creation, it has a chance to make sure the target size is right (inflate if necessary).

akalenyu · 2025-12-16T15:26:10Z

dv-hpp-cloned-from-datasource.yaml

It doesn't happen for me on upstream. Can you share your yaml file?

Yeah I don't see it either, may have to do with an older version

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.
I am a little confused that we have to inflate in the populator layer; usually we do this in the mutating pvc webhook (on by default)/dv controllers

Here you are: https://issues.redhat.com/browse/CNV-44140
Yes the issue happens at the very late stage where the snapshot clone falls back to host assisted clone.

I see, though I am still confused by inflating this far into the process. Doesn't the target PVC get inflated anyhow by the webhook/DV controller (and thus so does the temp)?

No the target PVC doesn't get inflated when doing snapshot clone. When the controller plans the clone it finds out the storage class is different from the volume snapshot's one and then it fall back to host assisted clone. When doing host clone phase it create the tmp target PVC, if we don't check here then it will report the failure because the target size is not enough (for this to happen the tmp target PVC volume mode must be filesystem).

I was pretty sure we inflate filesystem target PVCs here -

containerized-data-importer/pkg/controller/datavolume/util.go

Line 314 in e147bbd

func renderPvcSpecVolumeSize(client client.Client, pvcSpec *v1.PersistentVolumeClaimSpec, isClone bool, log *logr.Logger) error {

Sorry for poking I just think we have a problem elsewhere and that inflating the target in the populator will just cover that up

This method is never called in this case. Actually the webhook rendering is not enabled. For snapshot cloning it basically fallback to host-assisted during the planning phase, and when the host-clone phase is handling the temp target pvc creation, it has a chance to make sure the target size is right (inflate if necessary).

Webhook rendering is enabled since a few releases now, but anyway renderPvcSpecVolumeSize existed prior to it for the DV storage API. How come the target doesn't get inflated?

Acedus · 2025-12-17T12:58:29Z

I think the real issue here is that we don't inflate the PVC during mutation if it has a data source:

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 65 to 88 in e1bfde0

    
           func RenderPvc(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error { 
        
           	if pvc.Spec.VolumeMode != nil && 
        
           		*pvc.Spec.VolumeMode == cdiv1.PersistentVolumeFromStorageProfile { 
        
           		pvc.Spec.VolumeMode = nil 
        
           	} 
        
           	dvContentType := cc.GetPVCContentType(pvc) 
        
           	if err := renderPvcSpecVolumeModeAndAccessModesAndStorageClass(client, nil, nil, nil, &pvc.Spec, dvContentType); err != nil { 
        
           		return err 
        
           	} 
        
           	if hasCloneSourceRef(pvc) { 
        
           		return renderClonePvcVolumeSizeFromSource(ctx, client, pvc) 
        
           	} 
        
           	// Skip size inflation for PVCs restoring from VolumeSnapshots. 
        
           	// The PVC size must not exceed what the snapshot can provide; inflating it 
        
           	// causes strict CSI drivers (e.g., NetApp Trident) to reject the restore. 
        
           	if hasVolumeSnapshotDataSource(pvc) { 
        
           		return nil 
        
           	} 
        
           	return renderPvcSpecVolumeSize(client, &pvc.Spec, false, nil) 
        
           }

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 246 to 255 in e1bfde0

    
           if source.Kind == "VolumeSnapshot" && source.Name != "" { 
        
           	sourceSnapshot := &snapshotv1.VolumeSnapshot{} 
        
           	if err := client.Get(ctx, types.NamespacedName{Namespace: sourceNamespace, Name: source.Name}, sourceSnapshot); err != nil { 
        
           		return err 
        
           	} 
        
           	if sourceSnapshot.Status != nil && sourceSnapshot.Status.RestoreSize != nil { 
        
           		setRequestedVolumeSize(&pvc.Spec, *sourceSnapshot.Status.RestoreSize) 
        
           	} 
        
           	return nil 
        
           }

We simply set the restore size if it's available and leave it as is otherwise.

gaohoward · 2025-12-17T13:19:53Z

I think the real issue here is that we don't inflate the PVC during mutation if it has a data source:

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 65 to 88 in e1bfde0

func RenderPvc(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error {

if pvc.Spec.VolumeMode != nil &&

*pvc.Spec.VolumeMode == cdiv1.PersistentVolumeFromStorageProfile {

pvc.Spec.VolumeMode = nil

}

dvContentType := cc.GetPVCContentType(pvc)

if err := renderPvcSpecVolumeModeAndAccessModesAndStorageClass(client, nil, nil, nil, &pvc.Spec, dvContentType); err != nil {

return err

}

if hasCloneSourceRef(pvc) {

return renderClonePvcVolumeSizeFromSource(ctx, client, pvc)

}

// Skip size inflation for PVCs restoring from VolumeSnapshots.

// The PVC size must not exceed what the snapshot can provide; inflating it

// causes strict CSI drivers (e.g., NetApp Trident) to reject the restore.

if hasVolumeSnapshotDataSource(pvc) {

return nil

}

return renderPvcSpecVolumeSize(client, &pvc.Spec, false, nil)

}

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 246 to 255 in e1bfde0

if source.Kind == "VolumeSnapshot" && source.Name != "" {

sourceSnapshot := &snapshotv1.VolumeSnapshot{}

if err := client.Get(ctx, types.NamespacedName{Namespace: sourceNamespace, Name: source.Name}, sourceSnapshot); err != nil {

return err

}

if sourceSnapshot.Status != nil && sourceSnapshot.Status.RestoreSize != nil {

setRequestedVolumeSize(&pvc.Spec, *sourceSnapshot.Status.RestoreSize)

}

return nil

}

We simply set the restore size if it's available and leave it as is otherwise.

I rerun the test again and yes the webhook rendering is enabled. However the issue is that if the snapshot clone going as normal (doesn't fall back to host assisted) the target will get inflated as you said. But when it falls back to host clone, (clone strategy is copy), when the host-clone phase prepares the target tmp PVC, it didn't check the size
and do inflate if necessary. Here we added it so that the host clone won't get size problem.

containerized-data-importer/pkg/controller/populators/clone-populator.go

Line 315 in e1bfde0

csr, err := r.getCloneStrategy(ctx, log, pvc, vcs)

After this point the host clone takes over. If it doesn't fall back to host clone, the methods you mentioned get called and works. But if it go through host clone phase, we need this fix to take care of the size issue.

Acedus · 2025-12-17T14:04:48Z

I think the real issue here is that we don't inflate the PVC during mutation if it has a data source:

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 65 to 88 in e1bfde0

func RenderPvc(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error {

if pvc.Spec.VolumeMode != nil &&

*pvc.Spec.VolumeMode == cdiv1.PersistentVolumeFromStorageProfile {

pvc.Spec.VolumeMode = nil

}

dvContentType := cc.GetPVCContentType(pvc)

if err := renderPvcSpecVolumeModeAndAccessModesAndStorageClass(client, nil, nil, nil, &pvc.Spec, dvContentType); err != nil {

return err

}

if hasCloneSourceRef(pvc) {

return renderClonePvcVolumeSizeFromSource(ctx, client, pvc)

}

// Skip size inflation for PVCs restoring from VolumeSnapshots.

// The PVC size must not exceed what the snapshot can provide; inflating it

// causes strict CSI drivers (e.g., NetApp Trident) to reject the restore.

if hasVolumeSnapshotDataSource(pvc) {

return nil

}

return renderPvcSpecVolumeSize(client, &pvc.Spec, false, nil)

}

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 246 to 255 in e1bfde0

if source.Kind == "VolumeSnapshot" && source.Name != "" {

sourceSnapshot := &snapshotv1.VolumeSnapshot{}

if err := client.Get(ctx, types.NamespacedName{Namespace: sourceNamespace, Name: source.Name}, sourceSnapshot); err != nil {

return err

}

if sourceSnapshot.Status != nil && sourceSnapshot.Status.RestoreSize != nil {

setRequestedVolumeSize(&pvc.Spec, *sourceSnapshot.Status.RestoreSize)

}

return nil

}

We simply set the restore size if it's available and leave it as is otherwise.

I rerun the test again and yes the webhook rendering is enabled. However the issue is that if the snapshot clone going as normal (doesn't fall back to host assisted) the target will get inflated as you said. But when it falls back to host clone, (clone strategy is copy), when the host-clone phase prepares the target tmp PVC, it didn't check the size and do inflate if necessary. Here we added it so that the host clone won't get size problem.

containerized-data-importer/pkg/controller/populators/clone-populator.go

Line 315 in e1bfde0

csr, err := r.getCloneStrategy(ctx, log, pvc, vcs)

After this point the host clone takes over. If it doesn't fall back to host clone, the methods you mentioned get called and works. But if it go through host clone phase, we need this fix to take care of the size issue.

Even if it does fall back to host clone the target PVC is created so it should be mutated according to the logic I wrote above, shouldn't we fix that logic instead?

gaohoward · 2025-12-17T14:31:11Z

I think the real issue here is that we don't inflate the PVC during mutation if it has a data source:

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 65 to 88 in e1bfde0

func RenderPvc(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error {

if pvc.Spec.VolumeMode != nil &&

*pvc.Spec.VolumeMode == cdiv1.PersistentVolumeFromStorageProfile {

pvc.Spec.VolumeMode = nil

}

dvContentType := cc.GetPVCContentType(pvc)

if err := renderPvcSpecVolumeModeAndAccessModesAndStorageClass(client, nil, nil, nil, &pvc.Spec, dvContentType); err != nil {

return err

}

if hasCloneSourceRef(pvc) {

return renderClonePvcVolumeSizeFromSource(ctx, client, pvc)

}

// Skip size inflation for PVCs restoring from VolumeSnapshots.

// The PVC size must not exceed what the snapshot can provide; inflating it

// causes strict CSI drivers (e.g., NetApp Trident) to reject the restore.

if hasVolumeSnapshotDataSource(pvc) {

return nil

}

return renderPvcSpecVolumeSize(client, &pvc.Spec, false, nil)

}

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 246 to 255 in e1bfde0

if source.Kind == "VolumeSnapshot" && source.Name != "" {

sourceSnapshot := &snapshotv1.VolumeSnapshot{}

if err := client.Get(ctx, types.NamespacedName{Namespace: sourceNamespace, Name: source.Name}, sourceSnapshot); err != nil {

return err

}

if sourceSnapshot.Status != nil && sourceSnapshot.Status.RestoreSize != nil {

setRequestedVolumeSize(&pvc.Spec, *sourceSnapshot.Status.RestoreSize)

}

return nil

}

We simply set the restore size if it's available and leave it as is otherwise.

I rerun the test again and yes the webhook rendering is enabled. However the issue is that if the snapshot clone going as normal (doesn't fall back to host assisted) the target will get inflated as you said. But when it falls back to host clone, (clone strategy is copy), when the host-clone phase prepares the target tmp PVC, it didn't check the size and do inflate if necessary. Here we added it so that the host clone won't get size problem.

containerized-data-importer/pkg/controller/populators/clone-populator.go

Line 315 in e1bfde0

csr, err := r.getCloneStrategy(ctx, log, pvc, vcs)

After this point the host clone takes over. If it doesn't fall back to host clone, the methods you mentioned get called and works. But if it go through host clone phase, we need this fix to take care of the size issue.

Not sure that's another issue or not but this doesn't help the host assisted clone. The host clone will create a temp target PVC based on its clone source (in this case a volume snapshot). The temp targetPVC must satisfy the size requirement (infalted if necessary)

gaohoward · 2025-12-17T14:34:16Z

dv-hpp-cloned-from-datasource.yaml

It doesn't happen for me on upstream. Can you share your yaml file?

Yeah I don't see it either, may have to do with an older version

Thank you for tackling this! I would appreciate a link to the bug so I can do some testing and see the origin of the issue.
I am a little confused that we have to inflate in the populator layer; usually we do this in the mutating pvc webhook (on by default)/dv controllers

Here you are: https://issues.redhat.com/browse/CNV-44140
Yes the issue happens at the very late stage where the snapshot clone falls back to host assisted clone.

I see, though I am still confused by inflating this far into the process. Doesn't the target PVC get inflated anyhow by the webhook/DV controller (and thus so does the temp)?

No the target PVC doesn't get inflated when doing snapshot clone. When the controller plans the clone it finds out the storage class is different from the volume snapshot's one and then it fall back to host assisted clone. When doing host clone phase it create the tmp target PVC, if we don't check here then it will report the failure because the target size is not enough (for this to happen the tmp target PVC volume mode must be filesystem).

I was pretty sure we inflate filesystem target PVCs here -

containerized-data-importer/pkg/controller/datavolume/util.go

Line 314 in e147bbd

func renderPvcSpecVolumeSize(client client.Client, pvcSpec *v1.PersistentVolumeClaimSpec, isClone bool, log *logr.Logger) error {

Sorry for poking I just think we have a problem elsewhere and that inflating the target in the populator will just cover that up

This method is never called in this case. Actually the webhook rendering is not enabled. For snapshot cloning it basically fallback to host-assisted during the planning phase, and when the host-clone phase is handling the temp target pvc creation, it has a chance to make sure the target size is right (inflate if necessary).

Webhook rendering is enabled since a few releases now, but anyway renderPvcSpecVolumeSize existed prior to it for the DV storage API. How come the target doesn't get inflated?

Yes webhook rendering is enabled. Please see my reply to @Acedus for explanation why it is happening

akalenyu · 2025-12-17T15:07:26Z

I think the real issue here is that we don't inflate the PVC during mutation if it has a data source:

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 65 to 88 in e1bfde0

func RenderPvc(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error {

if pvc.Spec.VolumeMode != nil &&

*pvc.Spec.VolumeMode == cdiv1.PersistentVolumeFromStorageProfile {

pvc.Spec.VolumeMode = nil

}

dvContentType := cc.GetPVCContentType(pvc)

if err := renderPvcSpecVolumeModeAndAccessModesAndStorageClass(client, nil, nil, nil, &pvc.Spec, dvContentType); err != nil {

return err

}

if hasCloneSourceRef(pvc) {

return renderClonePvcVolumeSizeFromSource(ctx, client, pvc)

}

// Skip size inflation for PVCs restoring from VolumeSnapshots.

// The PVC size must not exceed what the snapshot can provide; inflating it

// causes strict CSI drivers (e.g., NetApp Trident) to reject the restore.

if hasVolumeSnapshotDataSource(pvc) {

return nil

}

return renderPvcSpecVolumeSize(client, &pvc.Spec, false, nil)

}

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 246 to 255 in e1bfde0

if source.Kind == "VolumeSnapshot" && source.Name != "" {

sourceSnapshot := &snapshotv1.VolumeSnapshot{}

if err := client.Get(ctx, types.NamespacedName{Namespace: sourceNamespace, Name: source.Name}, sourceSnapshot); err != nil {

return err

}

if sourceSnapshot.Status != nil && sourceSnapshot.Status.RestoreSize != nil {

setRequestedVolumeSize(&pvc.Spec, *sourceSnapshot.Status.RestoreSize)

}

return nil

}

We simply set the restore size if it's available and leave it as is otherwise.

This path is not exercised though if a size exists

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 225 to 228 in e1bfde0

    
           func renderClonePvcVolumeSizeFromSource(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error { 
        
           	if size, exists := pvc.Spec.Resources.Requests[v1.ResourceStorage]; exists && !size.IsZero() { 
        
           		return nil 
        
           	}

Acedus · 2025-12-17T15:15:30Z

I think the real issue here is that we don't inflate the PVC during mutation if it has a data source:

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 65 to 88 in e1bfde0

func RenderPvc(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error {

if pvc.Spec.VolumeMode != nil &&

*pvc.Spec.VolumeMode == cdiv1.PersistentVolumeFromStorageProfile {

pvc.Spec.VolumeMode = nil

}

dvContentType := cc.GetPVCContentType(pvc)

if err := renderPvcSpecVolumeModeAndAccessModesAndStorageClass(client, nil, nil, nil, &pvc.Spec, dvContentType); err != nil {

return err

}

if hasCloneSourceRef(pvc) {

return renderClonePvcVolumeSizeFromSource(ctx, client, pvc)

}

// Skip size inflation for PVCs restoring from VolumeSnapshots.

// The PVC size must not exceed what the snapshot can provide; inflating it

// causes strict CSI drivers (e.g., NetApp Trident) to reject the restore.

if hasVolumeSnapshotDataSource(pvc) {

return nil

}

return renderPvcSpecVolumeSize(client, &pvc.Spec, false, nil)

}

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 246 to 255 in e1bfde0

if source.Kind == "VolumeSnapshot" && source.Name != "" {

sourceSnapshot := &snapshotv1.VolumeSnapshot{}

if err := client.Get(ctx, types.NamespacedName{Namespace: sourceNamespace, Name: source.Name}, sourceSnapshot); err != nil {

return err

}

if sourceSnapshot.Status != nil && sourceSnapshot.Status.RestoreSize != nil {

setRequestedVolumeSize(&pvc.Spec, *sourceSnapshot.Status.RestoreSize)

}

return nil

}

We simply set the restore size if it's available and leave it as is otherwise.

This path is not exercised though if a size exists

containerized-data-importer/pkg/controller/datavolume/util.go

Lines 225 to 228 in e1bfde0

func renderClonePvcVolumeSizeFromSource(ctx context.Context, client client.Client, pvc *v1.PersistentVolumeClaim) error {

if size, exists := pvc.Spec.Resources.Requests[v1.ResourceStorage]; exists && !size.IsZero() {

return nil

}

~~Result is the same, if we would hit the latter path (renderPvcSpecVolumeSize) we'd correctly inflate the size.~~

EDIT: The target tmp PVC doesn't have a dataSource so in theory it should just hit the standard path and inflate properly.

when cloning from block to filesystem volume mode (kubevirt#3900) When cloning from a source, the host cloner should check the requested size for the target tmp pvc if the tmp source pvc is of block volume mode and the target pvc is of filesystem volume mode because it should take filesystem overhead into consideration and enlarge the original request size accordingly before creating the tmp pvc. Because currently the check is missing the cloning will fail on validation. Signed-off-by: Howard Gao <howard.gao@gmail.com>

kubevirt-bot · 2026-01-08T03:35:50Z

@gaohoward: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cdi-linter	`b606f62`	link	false	`/test pull-cdi-linter`

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 10, 2025

kubevirt-bot requested a review from aglitke September 10, 2025 13:39

kubevirt-bot requested a review from arnongilboa September 10, 2025 13:39

kubevirt-bot added the size/L label Sep 10, 2025

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 10, 2025

gaohoward force-pushed the cnv-44140 branch 2 times, most recently from 4942c4d to 3869e6a Compare September 15, 2025 02:53

gaohoward force-pushed the cnv-44140 branch 2 times, most recently from af4abed to 6a01cfa Compare October 13, 2025 09:13

awels reviewed Oct 22, 2025

View reviewed changes

pkg/controller/clone/host-clone.go Outdated Show resolved Hide resolved

gaohoward force-pushed the cnv-44140 branch 2 times, most recently from d00ad6c to 95f11c4 Compare October 23, 2025 08:53

gaohoward force-pushed the cnv-44140 branch from 95f11c4 to 761f2df Compare October 27, 2025 09:18

awels reviewed Nov 3, 2025

View reviewed changes

pkg/controller/clone/host-clone.go Outdated Show resolved Hide resolved

gaohoward force-pushed the cnv-44140 branch from 761f2df to d003e32 Compare November 6, 2025 13:35

gaohoward force-pushed the cnv-44140 branch from d003e32 to 191ad68 Compare November 11, 2025 12:27

gaohoward force-pushed the cnv-44140 branch from 7081344 to b894303 Compare December 2, 2025 03:31

awels reviewed Dec 3, 2025

View reviewed changes

gaohoward force-pushed the cnv-44140 branch 3 times, most recently from 379d8bd to c83e112 Compare December 8, 2025 11:50

gaohoward force-pushed the cnv-44140 branch from c83e112 to 8665110 Compare December 9, 2025 13:28

akalenyu reviewed Dec 10, 2025

View reviewed changes

gaohoward force-pushed the cnv-44140 branch from 8665110 to b606f62 Compare January 8, 2026 02:30

Host assisted clone should adjust requested storage when cloning from block to filesystem volume mode (#3900) #3901

Are you sure you want to change the base?

Host assisted clone should adjust requested storage when cloning from block to filesystem volume mode (#3900) #3901

Uh oh!

Conversation

gaohoward commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kubevirt-bot commented Sep 10, 2025

Uh oh!

kubevirt-bot commented Sep 10, 2025

Uh oh!

akalenyu commented Sep 11, 2025

Uh oh!

coveralls commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

awels commented Oct 13, 2025

Uh oh!

gaohoward commented Oct 13, 2025

Uh oh!

awels commented Oct 13, 2025

Uh oh!

gaohoward commented Oct 14, 2025

Uh oh!

Uh oh!

gaohoward commented Oct 23, 2025

Uh oh!

gaohoward commented Oct 23, 2025

Uh oh!

gaohoward commented Oct 27, 2025

Uh oh!

Uh oh!

gaohoward commented Nov 6, 2025

Uh oh!

gaohoward commented Dec 2, 2025

Uh oh!

gaohoward commented Dec 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaohoward commented Dec 8, 2025

Uh oh!

akalenyu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaohoward commented Dec 11, 2025

Uh oh!

akalenyu commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akalenyu commented Dec 11, 2025

Uh oh!

akalenyu commented Dec 11, 2025

Uh oh!

kubevirt-bot commented Dec 11, 2025

Uh oh!

gaohoward commented Dec 12, 2025

Uh oh!

gaohoward commented Dec 12, 2025

Uh oh!

akalenyu commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaohoward commented Dec 16, 2025

Uh oh!

akalenyu commented Dec 16, 2025

Uh oh!

Acedus commented Dec 17, 2025

Uh oh!

gaohoward commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaohoward commented Sep 10, 2025 •

edited

Loading

coveralls commented Sep 11, 2025 •

edited

Loading

akalenyu left a comment •

edited

Loading

akalenyu commented Dec 11, 2025 •

edited

Loading

akalenyu commented Dec 16, 2025 •

edited

Loading

gaohoward commented Dec 17, 2025 •

edited

Loading

Acedus commented Dec 17, 2025 •

edited

Loading